On Mon, Aug 7, 2017 at 3:43 PM, Scott D Phillips <scott.d.phill...@intel.com> wrote: > Matt Turner <matts...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
