On Thu, Jan 03, 2019 at 07:50:03AM +0100, Iago Toral wrote: > On Wed, 2019-01-02 at 11:35 +0200, Pohjolainen, Topi wrote: > > On Wed, Dec 19, 2018 at 12:50:56PM +0100, Iago Toral Quiroga wrote: > > > This is available since gen8. > > > --- > > > src/intel/compiler/brw_reg_type.c | 35 > > > +++++++++++++++++++++++++++---- > > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > > > diff --git a/src/intel/compiler/brw_reg_type.c > > > b/src/intel/compiler/brw_reg_type.c > > > index 60240ba1513..72295a2bd75 100644 > > > --- a/src/intel/compiler/brw_reg_type.c > > > +++ b/src/intel/compiler/brw_reg_type.c > > > @@ -138,6 +138,7 @@ enum hw_3src_reg_type { > > > GEN7_3SRC_TYPE_D = 1, > > > GEN7_3SRC_TYPE_UD = 2, > > > GEN7_3SRC_TYPE_DF = 3, > > > + GEN8_3SRC_TYPE_HF = 4, > > > > > > /** When ExecutionDatatype is 1: @{ */ > > > GEN10_ALIGN1_3SRC_REG_TYPE_HF = 0b000, > > > @@ -166,6 +167,14 @@ static const struct hw_3src_type { > > > [BRW_REGISTER_TYPE_D] = { GEN7_3SRC_TYPE_D }, > > > [BRW_REGISTER_TYPE_UD] = { GEN7_3SRC_TYPE_UD }, > > > [BRW_REGISTER_TYPE_DF] = { GEN7_3SRC_TYPE_DF }, > > > +}, gen8_hw_3src_type[] = { > > > + [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID }, > > > + > > > + [BRW_REGISTER_TYPE_F] = { GEN7_3SRC_TYPE_F }, > > > + [BRW_REGISTER_TYPE_D] = { GEN7_3SRC_TYPE_D }, > > > + [BRW_REGISTER_TYPE_UD] = { GEN7_3SRC_TYPE_UD }, > > > + [BRW_REGISTER_TYPE_DF] = { GEN7_3SRC_TYPE_DF }, > > > + [BRW_REGISTER_TYPE_HF] = { GEN8_3SRC_TYPE_HF }, > > > }, gen10_hw_3src_align1_type[] = { > > > #define E(x) BRW_ALIGN1_3SRC_EXEC_TYPE_##x > > > [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID }, > > > @@ -249,6 +258,20 @@ brw_hw_type_to_reg_type(const struct > > > gen_device_info *devinfo, > > > unreachable("not reached"); > > > } > > > > > > +static inline const struct hw_3src_type * > > > +get_hw_3src_type_map(const struct gen_device_info *devinfo, > > > uint32_t *size) > > > +{ > > > + if (devinfo->gen < 8) { > > > + if (size) > > > + *size = ARRAY_SIZE(gen7_hw_3src_type); > > > + return gen7_hw_3src_type; > > > + } else { > > > + if (size) > > > + *size = ARRAY_SIZE(gen8_hw_3src_type); > > > + return gen8_hw_3src_type; > > > + } > > > +} > > > + > > > /** > > > * Convert a brw_reg_type enumeration value into the hardware > > > representation > > > * for a 3-src align16 instruction > > > @@ -257,9 +280,11 @@ unsigned > > > brw_reg_type_to_a16_hw_3src_type(const struct gen_device_info > > > *devinfo, > > > enum brw_reg_type type) > > > { > > > - assert(type < ARRAY_SIZE(gen7_hw_3src_type)); > > > - assert(gen7_hw_3src_type[type].reg_type != (enum > > > hw_3src_reg_type)INVALID); > > > - return gen7_hw_3src_type[type].reg_type; > > > + uint32_t map_size; > > > + const struct hw_3src_type *hw_3src_type_map = > > > + get_hw_3src_type_map(devinfo, &map_size); > > > + assert(hw_3src_type_map[type].reg_type != (enum > > > hw_3src_reg_type)INVALID); > > > + return hw_3src_type_map[type].reg_type; > > > > I wonder if we should use a style equivalent to > > brw_reg_type_to_hw_type() and > > brw_hw_type_to_reg_type() and inline the table (or map) selection: > > I don't have a strong opinion, but since we need this in at least two > different places I think it is best to have that code in a single > function that we can reuse rather than replicating it wherever we need > it. I'd be more in favor of changing the other functions to follow a > similar pattern for the same reason.
I don't have a preference either. Having similar logic in both would be nice though. We could, for example, go with your patch here and as a follow-up modify the existing. Hence, this patch: Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > Iago > > > const struct hw_type *table; > > > > if (devinfo->gen >= 8) { > > assert(type < ARRAY_SIZE(gen8_hw_3src_type)); > > table = gen7_hw_3src_type; > > } else { > > assert(type < ARRAY_SIZE(gen7_hw_3src_type)); > > table = gen7_hw_3src_type; > > } > > > > assert(table[type].reg_type != (enum hw_reg_type)INVALID); > > > > return table[type].reg_type; > > > > > } > > > > > > /** > > > @@ -283,8 +308,10 @@ enum brw_reg_type > > > brw_a16_hw_3src_type_to_reg_type(const struct gen_device_info > > > *devinfo, > > > unsigned hw_type) > > > { > > > + const struct hw_3src_type *hw_3src_type_map = > > > + get_hw_3src_type_map(devinfo, NULL); > > > for (enum brw_reg_type i = 0; i <= BRW_REGISTER_TYPE_LAST; i++) > > > { > > > - if (gen7_hw_3src_type[i].reg_type == hw_type) { > > > + if (hw_3src_type_map[i].reg_type == hw_type) { > > > return i; > > > } > > > } > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > 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