On 20 November 2012 21:40, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Hey all, Here's a bunch of preliminary refactoring which should help me implement the Gen8 code generator. A bit of background: Gen8 uses a different instruction encoding than Gen4-7 (essentially, a lot of bits got moved around), which means that I can't use struct brw_instruction. This basically also means that the brw_eu_emit.c is useless to me, including the brw_compile struct with the store of brw_instruction structs. This patch series splits fs_visitor into two parts: - fs_visitor translates GLSL or Mesa IR into FS IR, optimizes it, and performs register allocation (it's still a giant monolith, sadly). This contains the emit() functions. - fs_generator generates the final assembly code, in a hardware specific format. This is the part that deals with brw_instruction and brw_eu_emit. This corresponds to the generate() layer and all the code that lives in the (poorly named) brw_fs_emit.cpp file. (We might want to rename that.) It actually generates both the SIMD8 and SIMD16 assembly together, as they need to be packed into a single program that can be uploaded to the GPU. Assembly generation can never fail. (The only failure mode was due to unsupported opcodes, which is an internal driver bug, not a user error.) For Gen8, I intend to replace fs_generator with a new backend, but reuse the fs_visitor frontends. A big reason for this refactoring is that my first attempt at a Gen8 code generator managed to emit a mix of Gen4-7 and Gen8 instructions. This is really easy to do, since brw_eu.h tends to get included all over the place, which gives you access to brw_instruction, emitter functions like brw_SAMPLE, and struct brw_compile, containing the instruction store. I want to ensure that C++ code which mixes encodings won't compile. By moving the brw_compile structure (commonly referred to as "p") into fs_generator, I manage to isolate it into the part I'm replacing, which means my new code won't have access to it, and thus can't mix encodings. Thanks in advance for the feedback. --Ken _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> I really like what you've done here, Ken. It's nice to see fs_visitor start to get broken down into smaller, more reusable components. I'm also very happy to see the use of private data members in the new fs_generator class. It gives me extra confidence that there aren't any hidden dependencies between fs_visitor and fs_generator that would get in the way of Gen8 code generation. The series is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> Looking forward to seeing the patch series you alluded to in patch 11/12 (making helper functions for generating instructions in both classes). </div>