[Mesa-dev] [PATCH 09/10] i965/gen8: Make disassembly function match brw's signature.

[Matt Turner]

On Tue, May 13, 2014 at 7:04 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 05/13/2014 02:52 PM, Matt Turner wrote:
>> gen8_dump_compile will be called indirectly by code common used by
>> generations before and after the gen8 instruction format change.
>> ---
>>  src/mesa/drivers/dri/i965/gen8_fs_generator.cpp   | 4 ++--
>>  src/mesa/drivers/dri/i965/gen8_generator.cpp      | 7 ++++---
>>  src/mesa/drivers/dri/i965/gen8_generator.h        | 6 ++++--
>>  src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp | 4 ++--
>>  4 files changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
>> index de06a97..d25da81 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
>> @@ -1293,7 +1293,7 @@ gen8_fs_generator::generate_code(exec_list *instructions)
>>        }
>>
>>        if (unlikely(INTEL_DEBUG & DEBUG_WM)) {
>> -         disassemble(stderr, last_native_inst_offset, next_inst_offset);
>> +         gen8_dump_compile(brw, store, last_native_inst_offset, next_inst_offset, stderr);
>
> I would much rather see both functions named "disassemble" than
> "dump_compile."  I think it's a better description of what the function
> does, and is really easy term to remember and search for.
>
> Perhaps:
> brw_disasm                  -> gen4_disassemble_inst
> gen8_disassemble            -> gen8_disassemble_inst
> brw_dump_compile            -> gen4_disassemble
> gen8_generator::disassemble -> gen8_disassemble
>
> Or disassemble_instruction/disassemble_program?
>
> What do you think?  I'm fine with leaving your patch as-is and renaming
> various functions in a follow-up patch or two.

Definitely agree. I had a hard time keeping them straight trying to
figure out which two of these needed to have the same interface.