[Mesa-dev] i965: Splitting out FS codegen ***re-autogen required***

[Paul Berry]

On 20 November 2012 21:40, Kenneth Graunke <kenneth at whitecape.org> wrote:

> 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

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 <stereotype441 at gmail.com>

Looking forward to seeing the patch series you alluded to in patch 11/12
(making helper functions for generating instructions in both classes).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121126/7f26ebcf/attachment.html>