On Fri, Sep 29, 2017 at 5:07 PM, Scott D Phillips
<scott.d.phill...@intel.com> wrote:
> Matt Turner <matts...@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.
I liked what I had until I implemented what you suggested. Your
suggestion (and then passing the result of
brw_inst_3src_a1_exec_type() directly to
brw_a1_hw_3src_type_to_reg_type() instead of "flags") simplifies all
of the callers a lot.
Thanks!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
