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

[Caio Marcelo de Oliveira Filho]

> >> +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.

Ah, got it. Consider adding a comment there about it.


> >> +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.

You read correctly my badly worded comment. Thanks.