[Mesa-dev] [PATCH 09/15] i965: Add a new representation for Broadwell shader instructions.

[Kenneth Graunke]

On 11/18/2013 03:22 PM, Kenneth Graunke wrote:
> On 11/18/2013 12:27 PM, Eric Anholt wrote:
>> Kenneth Graunke <kenneth at whitecape.org> writes:
>>
>>> Broadwell significantly changes the EU instruction encoding.  Many of
>>> the fields got moved to different bit positions; some even got split
>>> in two.
>>>
>>> With so many changes, it was infeasible to continue using struct
>>> brw_instruction.  We needed a new representation.
>>>
>>> This new approach is a bit different: rather than a struct, I created a
>>> class that has four DWords, and helper functions that read/write various
>>> bits.  This has several advantages:
>>>
>>> 1. We can create several different names for the same bits.  For
>>>    example, conditional modifiers, SFID for SEND instructions, and the
>>>    MATH instruction's function opcode are all stored in bits 27:24.
>>>
>>>    In each situation, we can use the appropriate setter function:
>>>    set_sfid(), set_math_function(), or set_cond_modifier().  This
>>>    is much easier to follow.
>>>
>>> 2. Since the fields are expressed using the original 128-bit numbers,
>>>    the code to create the getter/setter functions follows the table in
>>>    the documentation very closely.
>>>
>>> To aid in debugging, I've enabled -fkeep-inline-functions when building
>>> gen8_instruction.cpp.  Otherwise, these functions cannot be called by
>>> gdb, making it insanely difficult to print out anything.
>>
>> I dislike C++ creep.
> 
> I wrote this in C++ because all of the compiler infrastructure which
> uses it is already in C++.  It seemed natural.
> 
> However, Damien requested that we write it in C so it could be easily
> copied to xf86-video-intel, intel-gpu-tools, gen4asm/disasm, etc.  Which
> is probably a good thing.  He actually already ported it to C.

For reference, Damien's C port of this code is in the public
intel-gpu-tools repo as assember/gen8_instruction.[ch]:

http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/plain/assembler/gen8_instruction.h
http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/plain/assembler/gen8_instruction.c

So I guess we have some options:
1. Use the C++ version of my bitfield approach.
2. Use Damien's C port of my bitfield approach.
3. Rewrite it as a struct of union of structs again.

Having a system that Eric likes is important to me.  I'm interested to
hear other opinions as well...

--Ken