[Mesa-dev] [PATCH 20/23] intel/eu: Rework opcode description tables to allow efficient look-up by either HW or IR opcode.

[Francisco Jerez]

Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com> writes:

>> diff --git a/src/intel/Makefile.tools.am b/src/intel/Makefile.tools.am
>> index b00cc8cc2cb..4f2027cdfd6 100644
>> --- a/src/intel/Makefile.tools.am
>> +++ b/src/intel/Makefile.tools.am
>> @@ -27,6 +27,8 @@ tools_aubinator_SOURCES = \
>>  	tools/aubinator.c \
>>  	tools/intel_aub.h
>>  
>> +nodist_EXTRA_tools_aubinator_SOURCES = dummy.cpp
>> +
>>  tools_aubinator_CFLAGS = \
>>  	$(AM_CFLAGS) \
>>  	$(ZLIB_CFLAGS)
>> @@ -47,6 +49,8 @@ tools_aubinator_LDADD = \
>>  tools_aubinator_error_decode_SOURCES = \
>>  	tools/aubinator_error_decode.c
>>  
>> +nodist_EXTRA_tools_aubinator_error_decode_SOURCES = dummy.cpp
>> +
>>  tools_aubinator_error_decode_LDADD = \
>>  	common/libintel_common.la \
>>  	compiler/libintel_compiler.la \
>
> This hunk seems to be here by accident.
>

No, it was required to avoid a link failure in aubinator with the
autotools build, which is too stupid to realize it needs to use a C++
linker unless we add these dummy source files.

>
>> +static const opcode_desc *
>> +lookup_opcode_desc(gen_device_info *index_devinfo,
>> +                   const opcode_desc **index_descs,
>> +                   unsigned index_size,
>> +                   unsigned opcode_desc::*key,
>> +                   const gen_device_info *devinfo,
>> +                   unsigned k)
>>  {
>> -   if (opcode >= ARRAY_SIZE(opcode_descs))
>> -      return NULL;
>> -
>> -   enum gen gen = gen_from_devinfo(devinfo);
>> -   if (opcode_descs[opcode].gens != 0) {
>> -      if ((opcode_descs[opcode].gens & gen) != 0) {
>> -         return &opcode_descs[opcode];
>> -      }
>> -   } else if (opcode_descs[opcode].table != NULL) {
>> -      const struct opcode_desc *table = opcode_descs[opcode].table;
>> -      for (unsigned i = 0; i < opcode_descs[opcode].size; i++) {
>> -         if ((table[i].gens & gen) != 0) {
>> -            return &table[i];
>> +   if (memcmp(index_devinfo, devinfo, sizeof(*devinfo))) {
>
> Question: we are comparing a lot of bytes that seem irrelevant for
> invalidating the table. Is it a win versus what was being done before,
> i.e. invalidate by gen_device_info?
>

I'm not 100% clear what you mean by what was being done before since the
indexing code is new.  Are you suggesting to compare that
gen_from_devinfo(devinfo) == gen_from_devinfo(index_devinfo) instead of
comparing the devinfo structs directly?  Yes, that should work and be
slightly more efficient, since currently the index data structure only
depends on the gen number.  I've applied the optimization locally.

>
>
> Thanks,
> Caio
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180702/76834f04/attachment.sig>