Re: [Mesa-dev] [PATCH 14/25] i965: Use a common table to translate logical to hardware types
On Mon, Aug 7, 2017 at 3:43 PM, Scott D Phillipswrote: > Matt Turner 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
Re: [Mesa-dev] [PATCH 14/25] i965: Use a common table to translate logical to hardware types
Matt Turnerwrites: > --- > 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. > + > +static const struct { > + enum hw_reg_type reg_type; > + enum hw_imm_type imm_type; > +} gen4_hw_type[] = { > + [BRW_REGISTER_TYPE_DF] = { GEN7_HW_REG_TYPE_DF, GEN8_HW_IMM_TYPE_DF }, > + [BRW_REGISTER_TYPE_F] = { BRW_HW_REG_TYPE_F, BRW_HW_IMM_TYPE_F }, > + [BRW_REGISTER_TYPE_HF] = { GEN8_HW_REG_TYPE_HF, GEN8_HW_IMM_TYPE_HF }, > + [BRW_REGISTER_TYPE_VF] = { INVALID, BRW_HW_IMM_TYPE_VF }, > + > + [BRW_REGISTER_TYPE_Q] = { GEN8_HW_REG_TYPE_Q, GEN8_HW_IMM_TYPE_Q }, > + [BRW_REGISTER_TYPE_UQ] = { GEN8_HW_REG_TYPE_UQ, GEN8_HW_IMM_TYPE_UQ }, > + [BRW_REGISTER_TYPE_D] = { BRW_HW_REG_TYPE_D, BRW_HW_IMM_TYPE_D }, > + [BRW_REGISTER_TYPE_UD] = { BRW_HW_REG_TYPE_UD, BRW_HW_IMM_TYPE_UD }, > + [BRW_REGISTER_TYPE_W] = { BRW_HW_REG_TYPE_W, BRW_HW_IMM_TYPE_W }, > + [BRW_REGISTER_TYPE_UW] = { BRW_HW_REG_TYPE_UW, BRW_HW_IMM_TYPE_UW }, > + [BRW_REGISTER_TYPE_B] = { BRW_HW_REG_TYPE_B, INVALID }, > + [BRW_REGISTER_TYPE_UB] = { BRW_HW_REG_TYPE_UB, INVALID }, > + [BRW_REGISTER_TYPE_V] = { INVALID, BRW_HW_IMM_TYPE_V }, > + [BRW_REGISTER_TYPE_UV] = { INVALID, BRW_HW_IMM_TYPE_UV }, > +}; > + > /** > * Convert a brw_reg_type enumeration value into the hardware representation. > * > @@ -35,44 +58,14 @@ brw_reg_type_to_hw_type(const struct gen_device_info > *devinfo, > enum brw_reg_file file, > enum brw_reg_type type) > { > + assert(type < ARRAY_SIZE(gen4_hw_type)); > + > if (file == BRW_IMMEDIATE_VALUE) { > - static const enum hw_imm_type hw_types[] = { > - [0 ... BRW_REGISTER_TYPE_LAST] = -1, > - [BRW_REGISTER_TYPE_UD] = BRW_HW_IMM_TYPE_UD, > - [BRW_REGISTER_TYPE_D] = BRW_HW_IMM_TYPE_D, > - [BRW_REGISTER_TYPE_UW] = BRW_HW_IMM_TYPE_UW, > - [BRW_REGISTER_TYPE_W] = BRW_HW_IMM_TYPE_W, > - [BRW_REGISTER_TYPE_F] = BRW_HW_IMM_TYPE_F, > - [BRW_REGISTER_TYPE_UV] = BRW_HW_IMM_TYPE_UV, > - [BRW_REGISTER_TYPE_VF] = BRW_HW_IMM_TYPE_VF, > - [BRW_REGISTER_TYPE_V] = BRW_HW_IMM_TYPE_V, > - [BRW_REGISTER_TYPE_DF] = GEN8_HW_IMM_TYPE_DF, > - [BRW_REGISTER_TYPE_HF] = GEN8_HW_IMM_TYPE_HF, > - [BRW_REGISTER_TYPE_UQ] = GEN8_HW_IMM_TYPE_UQ, > - [BRW_REGISTER_TYPE_Q] = GEN8_HW_IMM_TYPE_Q, > - }; > - assert(type < ARRAY_SIZE(hw_types)); > - assert(hw_types[type] != -1); > - return hw_types[type]; > + assert(gen4_hw_type[type].imm_type != INVALID); > + return gen4_hw_type[type].imm_type; > } else { > - /* Non-immediate registers */ > - static const enum hw_reg_type hw_types[] = { > - [0 ... BRW_REGISTER_TYPE_LAST] = -1, > - [BRW_REGISTER_TYPE_UD] = BRW_HW_REG_TYPE_UD, > - [BRW_REGISTER_TYPE_D] = BRW_HW_REG_TYPE_D, > - [BRW_REGISTER_TYPE_UW] = BRW_HW_REG_TYPE_UW, > - [BRW_REGISTER_TYPE_W] = BRW_HW_REG_TYPE_W, > - [BRW_REGISTER_TYPE_UB] = BRW_HW_REG_TYPE_UB, > - [BRW_REGISTER_TYPE_B] = BRW_HW_REG_TYPE_B, > - [BRW_REGISTER_TYPE_F] = BRW_HW_REG_TYPE_F, > - [BRW_REGISTER_TYPE_DF] = GEN7_HW_REG_TYPE_DF, > - [BRW_REGISTER_TYPE_HF] = GEN8_HW_REG_TYPE_HF, > - [BRW_REGISTER_TYPE_UQ] = GEN8_HW_REG_TYPE_UQ, > - [BRW_REGISTER_TYPE_Q] = GEN8_HW_REG_TYPE_Q, > - }; > - assert(type < ARRAY_SIZE(hw_types)); > - assert(hw_types[type] != -1); > - return hw_types[type]; > + assert(gen4_hw_type[type].reg_type != INVALID); > + return gen4_hw_type[type].reg_type; > } > } > > -- > 2.13.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 14/25] i965: Use a common table to translate logical to hardware types
--- 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) + +static const struct { + enum hw_reg_type reg_type; + enum hw_imm_type imm_type; +} gen4_hw_type[] = { + [BRW_REGISTER_TYPE_DF] = { GEN7_HW_REG_TYPE_DF, GEN8_HW_IMM_TYPE_DF }, + [BRW_REGISTER_TYPE_F] = { BRW_HW_REG_TYPE_F, BRW_HW_IMM_TYPE_F }, + [BRW_REGISTER_TYPE_HF] = { GEN8_HW_REG_TYPE_HF, GEN8_HW_IMM_TYPE_HF }, + [BRW_REGISTER_TYPE_VF] = { INVALID, BRW_HW_IMM_TYPE_VF }, + + [BRW_REGISTER_TYPE_Q] = { GEN8_HW_REG_TYPE_Q, GEN8_HW_IMM_TYPE_Q }, + [BRW_REGISTER_TYPE_UQ] = { GEN8_HW_REG_TYPE_UQ, GEN8_HW_IMM_TYPE_UQ }, + [BRW_REGISTER_TYPE_D] = { BRW_HW_REG_TYPE_D, BRW_HW_IMM_TYPE_D }, + [BRW_REGISTER_TYPE_UD] = { BRW_HW_REG_TYPE_UD, BRW_HW_IMM_TYPE_UD }, + [BRW_REGISTER_TYPE_W] = { BRW_HW_REG_TYPE_W, BRW_HW_IMM_TYPE_W }, + [BRW_REGISTER_TYPE_UW] = { BRW_HW_REG_TYPE_UW, BRW_HW_IMM_TYPE_UW }, + [BRW_REGISTER_TYPE_B] = { BRW_HW_REG_TYPE_B, INVALID }, + [BRW_REGISTER_TYPE_UB] = { BRW_HW_REG_TYPE_UB, INVALID }, + [BRW_REGISTER_TYPE_V] = { INVALID, BRW_HW_IMM_TYPE_V }, + [BRW_REGISTER_TYPE_UV] = { INVALID, BRW_HW_IMM_TYPE_UV }, +}; + /** * Convert a brw_reg_type enumeration value into the hardware representation. * @@ -35,44 +58,14 @@ brw_reg_type_to_hw_type(const struct gen_device_info *devinfo, enum brw_reg_file file, enum brw_reg_type type) { + assert(type < ARRAY_SIZE(gen4_hw_type)); + if (file == BRW_IMMEDIATE_VALUE) { - static const enum hw_imm_type hw_types[] = { - [0 ... BRW_REGISTER_TYPE_LAST] = -1, - [BRW_REGISTER_TYPE_UD] = BRW_HW_IMM_TYPE_UD, - [BRW_REGISTER_TYPE_D] = BRW_HW_IMM_TYPE_D, - [BRW_REGISTER_TYPE_UW] = BRW_HW_IMM_TYPE_UW, - [BRW_REGISTER_TYPE_W] = BRW_HW_IMM_TYPE_W, - [BRW_REGISTER_TYPE_F] = BRW_HW_IMM_TYPE_F, - [BRW_REGISTER_TYPE_UV] = BRW_HW_IMM_TYPE_UV, - [BRW_REGISTER_TYPE_VF] = BRW_HW_IMM_TYPE_VF, - [BRW_REGISTER_TYPE_V] = BRW_HW_IMM_TYPE_V, - [BRW_REGISTER_TYPE_DF] = GEN8_HW_IMM_TYPE_DF, - [BRW_REGISTER_TYPE_HF] = GEN8_HW_IMM_TYPE_HF, - [BRW_REGISTER_TYPE_UQ] = GEN8_HW_IMM_TYPE_UQ, - [BRW_REGISTER_TYPE_Q] = GEN8_HW_IMM_TYPE_Q, - }; - assert(type < ARRAY_SIZE(hw_types)); - assert(hw_types[type] != -1); - return hw_types[type]; + assert(gen4_hw_type[type].imm_type != INVALID); + return gen4_hw_type[type].imm_type; } else { - /* Non-immediate registers */ - static const enum hw_reg_type hw_types[] = { - [0 ... BRW_REGISTER_TYPE_LAST] = -1, - [BRW_REGISTER_TYPE_UD] = BRW_HW_REG_TYPE_UD, - [BRW_REGISTER_TYPE_D] = BRW_HW_REG_TYPE_D, - [BRW_REGISTER_TYPE_UW] = BRW_HW_REG_TYPE_UW, - [BRW_REGISTER_TYPE_W] = BRW_HW_REG_TYPE_W, - [BRW_REGISTER_TYPE_UB] = BRW_HW_REG_TYPE_UB, - [BRW_REGISTER_TYPE_B] = BRW_HW_REG_TYPE_B, - [BRW_REGISTER_TYPE_F] = BRW_HW_REG_TYPE_F, - [BRW_REGISTER_TYPE_DF] = GEN7_HW_REG_TYPE_DF, - [BRW_REGISTER_TYPE_HF] = GEN8_HW_REG_TYPE_HF, - [BRW_REGISTER_TYPE_UQ] = GEN8_HW_REG_TYPE_UQ, - [BRW_REGISTER_TYPE_Q] = GEN8_HW_REG_TYPE_Q, - }; - assert(type < ARRAY_SIZE(hw_types)); - assert(hw_types[type] != -1); - return hw_types[type]; + assert(gen4_hw_type[type].reg_type != INVALID); + return gen4_hw_type[type].reg_type; } } -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev