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

[Kenneth Graunke]

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.

I can dig up that version and see how you like it...

> I think the old structs worked OK, and there are
> some minor downsides to access-everything-through-methods, like being
> unable to just print the instruction in gdb and see fields all at once,

Huh.  I've never done that.  Is that something you actually do?  I
imagine looking at the raw dump would be pretty huge, and a
pretty-printer/disassembler would always be more useful...

> and having to do the build system hack to inline but also keep the
> inlines.  But I'm feeling more and more alone on the team.

Well, you're less alone than you think.

I believe the build system hack is fairly orthogonal to the C++ issue.
We could also have small inline C functions defined in a .h file which
also take the get_bits/set_bits approach.

I would have appreciated getting "I don't like your design" feedback
sooner than a year after I wrote it.  But the most important thing is to
have the system we want going forward, which people are happy with and
can maintain.  So I'm open to rewriting it (just a bit grumpy).

--Ken