[Mesa-dev] [PATCH 08/13] i965: Add align1 ternary instruction support to conversion functions

Scott D Phillips scott.d.phillips at intel.com
Sat Sep 30 00:07:00 UTC 2017


Matt Turner <mattst88 at gmail.com> writes:

> ---
>  src/intel/compiler/brw_disasm.c   | 12 ++++---
>  src/intel/compiler/brw_inst.h     |  4 +--
>  src/intel/compiler/brw_reg_type.c | 76 ++++++++++++++++++++++++++++++++-------
>  src/intel/compiler/brw_reg_type.h |  7 ++--
>  4 files changed, 79 insertions(+), 20 deletions(-)
>
> diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
> index aab4a65b7d..3726172e5d 100644
> --- a/src/intel/compiler/brw_disasm.c
> +++ b/src/intel/compiler/brw_disasm.c
> @@ -766,7 +766,8 @@ dest_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins
>     uint32_t reg_file;
>     enum brw_reg_type type =
>        brw_hw_3src_type_to_reg_type(devinfo,
> -                                   brw_inst_3src_a16_dst_hw_type(devinfo, inst));
> +                                   brw_inst_3src_a16_dst_hw_type(devinfo, inst),
> +                                   0);
>     unsigned dst_subreg_nr =
>        brw_inst_3src_a16_dst_subreg_nr(devinfo, inst) * 4 /
>        brw_reg_type_to_size(type);
> @@ -936,7 +937,8 @@ src0_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins
>     int err = 0;
>     enum brw_reg_type type =
>        brw_hw_3src_type_to_reg_type(devinfo,
> -                                   brw_inst_3src_a16_src_hw_type(devinfo, inst));
> +                                   brw_inst_3src_a16_src_hw_type(devinfo, inst),
> +                                   0);
>     unsigned src0_subreg_nr =
>        brw_inst_3src_a16_src0_subreg_nr(devinfo, inst) * 4 /
>        brw_reg_type_to_size(type);
> @@ -968,7 +970,8 @@ src1_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins
>     int err = 0;
>     enum brw_reg_type type =
>        brw_hw_3src_type_to_reg_type(devinfo,
> -                                   brw_inst_3src_a16_src_hw_type(devinfo, inst));
> +                                   brw_inst_3src_a16_src_hw_type(devinfo, inst),
> +                                   0);
>     unsigned src1_subreg_nr =
>        brw_inst_3src_a16_src1_subreg_nr(devinfo, inst) * 4 /
>        brw_reg_type_to_size(type);
> @@ -1001,7 +1004,8 @@ src2_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins
>     int err = 0;
>     enum brw_reg_type type =
>        brw_hw_3src_type_to_reg_type(devinfo,
> -                                   brw_inst_3src_a16_src_hw_type(devinfo, inst));
> +                                   brw_inst_3src_a16_src_hw_type(devinfo, inst),
> +                                   0);
>     unsigned src2_subreg_nr =
>        brw_inst_3src_a16_src2_subreg_nr(devinfo, inst) * 4 /
>        brw_reg_type_to_size(type);
> diff --git a/src/intel/compiler/brw_inst.h b/src/intel/compiler/brw_inst.h
> index 0cc1a3e911..e6169057e3 100644
> --- a/src/intel/compiler/brw_inst.h
> +++ b/src/intel/compiler/brw_inst.h
> @@ -251,7 +251,7 @@ static inline void                                                            \
>  brw_inst_set_3src_a16_##reg##_type(const struct gen_device_info *devinfo,     \
>                                     brw_inst *inst, enum brw_reg_type type)    \
>  {                                                                             \
> -   unsigned hw_type = brw_reg_type_to_hw_3src_type(devinfo, type);            \
> +   unsigned hw_type = brw_reg_type_to_hw_3src_type(devinfo, type, 0);         \
>     brw_inst_set_3src_a16_##reg##_hw_type(devinfo, inst, hw_type);             \
>  }                                                                             \
>                                                                                \
> @@ -260,7 +260,7 @@ brw_inst_3src_a16_##reg##_type(const struct gen_device_info *devinfo,         \
>                                 const brw_inst *inst)                          \
>  {                                                                             \
>     unsigned hw_type = brw_inst_3src_a16_##reg##_hw_type(devinfo, inst);       \
> -   return brw_hw_3src_type_to_reg_type(devinfo, hw_type);                     \
> +   return brw_hw_3src_type_to_reg_type(devinfo, hw_type, 0);                  \
>  }
>  
>  REG_TYPE(dst)
> diff --git a/src/intel/compiler/brw_reg_type.c b/src/intel/compiler/brw_reg_type.c
> index d65ebaee48..7fb4a1e62a 100644
> --- a/src/intel/compiler/brw_reg_type.c
> +++ b/src/intel/compiler/brw_reg_type.c
> @@ -84,20 +84,55 @@ static const struct {
>   * and unsigned doublewords, so a new field is also available in the da3src
>   * struct (part of struct brw_instruction.bits1 in brw_structs.h) to select
>   * dst and shared-src types.
> + *
> + * CNL adds support for 3-src instructions in align1 mode, and with it support
> + * for most register types.
>   */
>  enum hw_3src_reg_type {
>     GEN7_3SRC_TYPE_F  = 0,
>     GEN7_3SRC_TYPE_D  = 1,
>     GEN7_3SRC_TYPE_UD = 2,
>     GEN7_3SRC_TYPE_DF = 3,
> +
> +   /** When ExecutionDatatype is 1: @{ */
> +   GEN10_ALIGN1_3SRC_REG_TYPE_HF = 0b000,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_F  = 0b001,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_DF = 0b010,
> +   /** @} */
> +
> +   /** When ExecutionDatatype is 0: @{ */
> +   GEN10_ALIGN1_3SRC_REG_TYPE_UD = 0b000,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_D  = 0b001,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_UW = 0b010,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_W  = 0b011,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_UB = 0b100,
> +   GEN10_ALIGN1_3SRC_REG_TYPE_B  = 0b101,
> +   /** @} */
>  };
>  
> -static const enum hw_3src_reg_type gen7_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,
> +static const struct hw_3src_type {
> +   enum hw_3src_reg_type reg_type;
> +   bool is_integer;
> +} gen7_hw_3src_type[] = {
> +   [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID,   false  },
> +
> +   [BRW_REGISTER_TYPE_F]  = { GEN7_3SRC_TYPE_F,  false  },
> +   [BRW_REGISTER_TYPE_D]  = { GEN7_3SRC_TYPE_D,  true,  },
> +   [BRW_REGISTER_TYPE_UD] = { GEN7_3SRC_TYPE_UD, true,  },
> +   [BRW_REGISTER_TYPE_DF] = { GEN7_3SRC_TYPE_DF, false, },
> +}, gen10_hw_3src_align1_type[] = {
> +   [0 ... BRW_REGISTER_TYPE_LAST] = { INVALID,   false  },
> +
> +   [BRW_REGISTER_TYPE_DF] = { GEN10_ALIGN1_3SRC_REG_TYPE_DF, false, },
> +   [BRW_REGISTER_TYPE_F]  = { GEN10_ALIGN1_3SRC_REG_TYPE_F,  false, },
> +   [BRW_REGISTER_TYPE_HF] = { GEN10_ALIGN1_3SRC_REG_TYPE_HF, false, },
> +
> +   [BRW_REGISTER_TYPE_D]  = { GEN10_ALIGN1_3SRC_REG_TYPE_D,  true,  },
> +   [BRW_REGISTER_TYPE_UD] = { GEN10_ALIGN1_3SRC_REG_TYPE_UD, true,  },
> +   [BRW_REGISTER_TYPE_W]  = { GEN10_ALIGN1_3SRC_REG_TYPE_W,  true,  },
> +   [BRW_REGISTER_TYPE_UW] = { GEN10_ALIGN1_3SRC_REG_TYPE_UW, true,  },
> +   [BRW_REGISTER_TYPE_B]  = { GEN10_ALIGN1_3SRC_REG_TYPE_B,  true,  },
> +   [BRW_REGISTER_TYPE_UB] = { GEN10_ALIGN1_3SRC_REG_TYPE_UB, true,  },
>  };
>  
>  /**
> @@ -152,11 +187,19 @@ brw_hw_type_to_reg_type(const struct gen_device_info *devinfo,
>   */
>  unsigned
>  brw_reg_type_to_hw_3src_type(const struct gen_device_info *devinfo,

I think it would make more sense to make separate functions for a16 and
a1 hw_type instead of using the flag field. I think that matches the way
they're listed as separate encodings in the spec.

> -                             enum brw_reg_type type)
> +                             enum brw_reg_type type, unsigned flags)
>  {
> -   assert(type < ARRAY_SIZE(gen7_3src_type));
> -   assert(gen7_3src_type[type] != -1);
> -   return gen7_3src_type[type];
> +   const struct hw_3src_type *table;
> +
> +   if ((flags & IS_ALIGN1) != 0) {
> +      assert(type < ARRAY_SIZE(gen10_hw_3src_align1_type));
> +      table = gen10_hw_3src_align1_type;
> +   } else {
> +      assert(type < ARRAY_SIZE(gen7_hw_3src_type));
> +      table = gen7_hw_3src_type;
> +   }
> +   assert(table[type].reg_type != INVALID);
> +   return table[type].reg_type;
>  }
>  
>  /**
> @@ -165,10 +208,19 @@ brw_reg_type_to_hw_3src_type(const struct gen_device_info *devinfo,
>   */
>  enum brw_reg_type
>  brw_hw_3src_type_to_reg_type(const struct gen_device_info *devinfo,
> -                             unsigned hw_type)
> +                             unsigned hw_type, unsigned flags)
>  {
> +   const struct hw_3src_type *table;
> +
> +   if ((flags & IS_ALIGN1) != 0) {
> +      table = gen10_hw_3src_align1_type;
> +   } else {
> +      table = gen7_hw_3src_type;
> +   }
> +
>     for (enum brw_reg_type i = 0; i <= BRW_REGISTER_TYPE_LAST; i++) {
> -      if (gen7_3src_type[i] == hw_type) {
> +      if (table[i].reg_type == hw_type &&
> +          table[i].is_integer == ((flags & IS_INTEGER) != 0)) {
>           return i;
>        }
>     }
> diff --git a/src/intel/compiler/brw_reg_type.h b/src/intel/compiler/brw_reg_type.h
> index ed249d77e6..c068b022df 100644
> --- a/src/intel/compiler/brw_reg_type.h
> +++ b/src/intel/compiler/brw_reg_type.h
> @@ -88,13 +88,16 @@ enum brw_reg_type ATTRIBUTE_PURE
>  brw_hw_type_to_reg_type(const struct gen_device_info *devinfo,
>                          enum brw_reg_file file, unsigned hw_type);
>  
> +#define IS_ALIGN1  0x1
> +#define IS_INTEGER 0x2
> +
>  unsigned
>  brw_reg_type_to_hw_3src_type(const struct gen_device_info *devinfo,
> -                             enum brw_reg_type type);
> +                             enum brw_reg_type type, unsigned flags);
>  
>  enum brw_reg_type
>  brw_hw_3src_type_to_reg_type(const struct gen_device_info *devinfo,
> -                             unsigned hw_type);
> +                             unsigned hw_type, unsigned flags);
>  
>  unsigned
>  brw_reg_type_to_size(enum brw_reg_type type);
> -- 
> 2.13.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list