[Mesa-dev] [PATCH 14/25] i965: Use a common table to translate logical to hardware types

[Matt Turner]

On Mon, Aug 7, 2017 at 3:43 PM, Scott D Phillips
<scott.d.phillips at intel.com> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>> ---
>>  src/intel/compiler/brw_reg_type.c | 65 +++++++++++++++++----------------------
>>  1 file changed, 29 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/intel/compiler/brw_reg_type.c b/src/intel/compiler/brw_reg_type.c
>> index 8aac0ca009..b0696570e5 100644
>> --- a/src/intel/compiler/brw_reg_type.c
>> +++ b/src/intel/compiler/brw_reg_type.c
>> @@ -25,6 +25,29 @@
>>  #include "brw_eu_defines.h"
>>  #include "common/gen_device_info.h"
>>
>> +#define INVALID (-1)
>
> The reg and imm enums have only non-negative values, so the compiler
> could choose an underlying type that is unsigned. The compiler could
> then elide the assert checks against INVALID as impossible because the
> type is unsigned. I guess the code is effectively the same as before,
> just noticed the warning from clang while looking at the patch.

Thanks. I definitely would not have thought about this.

We discussed this on IRC a bit, and ultimately concluded that casting
to the enum type in the assertion was the best approach:

   assert(gen4_hw_type[type].imm_type != (enum hw_imm_type)INVALID);

I tried defining INVALID as 0xff -- the thinking being that regardless
of the size of the underlying type it should be representable, but
clang gave the same warning as before. I also tried putting the cast
directly in the INVALID macro, but the two assertions compare it
against different enum values.