[Mesa-dev] [PATCH 10/13] i965: Add align1 ternary instruction disassembler support

Matt Turner mattst88 at gmail.com
Thu Oct 19 23:19:48 UTC 2017


On Fri, Sep 29, 2017 at 5:11 PM, Scott D Phillips
<scott.d.phillips at intel.com> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> ---
>>  src/intel/compiler/brw_disasm.c     | 399 +++++++++++++++++++++++++++++-------
>>  src/intel/compiler/brw_eu_defines.h |  11 -
>>  2 files changed, 322 insertions(+), 88 deletions(-)
>>
>> diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
>> index 3726172e5d..7215735967 100644
>> --- a/src/intel/compiler/brw_disasm.c
>> +++ b/src/intel/compiler/brw_disasm.c
>> @@ -30,6 +30,7 @@
>>  #include "brw_reg.h"
>>  #include "brw_inst.h"
>>  #include "brw_eu.h"
>> +#include "util/half_float.h"
>>
>>  static bool
>>  has_jip(const struct gen_device_info *devinfo, enum opcode opcode)
>> @@ -237,13 +238,6 @@ static const char *const access_mode[2] = {
>>     [1] = "align16",
>>  };
>>
>> -static const char *const three_source_reg_encoding[] = {
>> -   [BRW_3SRC_TYPE_F]  = "F",
>> -   [BRW_3SRC_TYPE_D]  = "D",
>> -   [BRW_3SRC_TYPE_UD] = "UD",
>> -   [BRW_3SRC_TYPE_DF] = "DF",
>> -};
>> -
>>  static const char *const reg_file[4] = {
>>     [0] = "A",
>>     [1] = "g",
>> @@ -762,17 +756,17 @@ dest(FILE *file, const struct gen_device_info *devinfo, const brw_inst *inst)
>>  static int
>>  dest_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *inst)
>>  {
>> +   bool is_align1 = brw_inst_3src_access_mode(devinfo, inst) == BRW_ALIGN_1;
>>     int err = 0;
>> +   unsigned flags = 0;
>>     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),
>> -                                   0);
>> -   unsigned dst_subreg_nr =
>> -      brw_inst_3src_a16_dst_subreg_nr(devinfo, inst) * 4 /
>> -      brw_reg_type_to_size(type);
>> -
>> -   if (devinfo->gen == 6 && brw_inst_3src_a16_dst_reg_file(devinfo, inst))
>> +   unsigned subreg_nr;
>> +   unsigned hw_type;
>> +   enum brw_reg_type type;
>> +
>> +   if (is_align1 && brw_inst_3src_a1_dst_reg_file(devinfo, inst))
>> +      reg_file = BRW_ARCHITECTURE_REGISTER_FILE;
>> +   else if (devinfo->gen == 6 && brw_inst_3src_a16_dst_reg_file(devinfo, inst))
>>        reg_file = BRW_MESSAGE_REGISTER_FILE;
>>     else
>>        reg_file = BRW_GENERAL_REGISTER_FILE;
>> @@ -780,13 +774,32 @@ dest_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins
>>     err |= reg(file, reg_file, brw_inst_3src_dst_reg_nr(devinfo, inst));
>>     if (err == -1)
>>        return 0;
>> -   if (dst_subreg_nr)
>> -      format(file, ".%u", dst_subreg_nr);
>> +
>> +   if (is_align1) {
>> +      flags |= IS_ALIGN1;
>> +
>> +      if (brw_inst_3src_a1_exec_type(devinfo, inst) ==
>> +          BRW_ALIGN1_3SRC_EXEC_TYPE_INT)
>> +         flags |= IS_INTEGER;
>> +
>> +      hw_type = brw_inst_3src_a1_dst_hw_type(devinfo, inst);
>> +      subreg_nr = brw_inst_3src_a1_dst_subreg_nr(devinfo, inst);
>> +   } else {
>> +      hw_type = brw_inst_3src_a16_dst_hw_type(devinfo, inst);
>> +      subreg_nr = brw_inst_3src_a16_dst_subreg_nr(devinfo, inst) * 4;
>> +   }
>> +   type = brw_hw_3src_type_to_reg_type(devinfo, hw_type, flags);
>> +   subreg_nr /= brw_reg_type_to_size(type);
>> +
>> +   if (subreg_nr)
>> +      format(file, ".%u", subreg_nr);
>>     string(file, "<1>");
>> -   err |= control(file, "writemask", writemask,
>> -                  brw_inst_3src_a16_dst_writemask(devinfo, inst), NULL);
>> -   err |= control(file, "dest reg encoding", three_source_reg_encoding,
>> -                  brw_inst_3src_a16_dst_hw_type(devinfo, inst), NULL);
>> +
>> +   if (!is_align1) {
>> +      err |= control(file, "writemask", writemask,
>> +                     brw_inst_3src_a16_dst_writemask(devinfo, inst), NULL);
>> +   }
>> +   string(file, brw_reg_type_to_letters(type));
>>
>>     return 0;
>>  }
>> @@ -931,36 +944,169 @@ src_da16(FILE *file,
>>     return err;
>>  }
>>
>> +static enum brw_vertical_stride
>> +vstride_from_align1_3src_vstride(enum gen10_align1_3src_vertical_stride vstride)
>> +{
>> +   switch (vstride) {
>> +   case BRW_ALIGN1_3SRC_VERTICAL_STRIDE_0: return BRW_VERTICAL_STRIDE_0;
>> +   case BRW_ALIGN1_3SRC_VERTICAL_STRIDE_2: return BRW_VERTICAL_STRIDE_2;
>> +   case BRW_ALIGN1_3SRC_VERTICAL_STRIDE_4: return BRW_VERTICAL_STRIDE_4;
>> +   case BRW_ALIGN1_3SRC_VERTICAL_STRIDE_8: return BRW_VERTICAL_STRIDE_8;
>> +   default:
>> +      unreachable("not reached");
>> +   }
>> +}
>> +
>> +static enum brw_horizontal_stride
>> +hstride_from_align1_3src_hstride(enum gen10_align1_3src_src_horizontal_stride hstride)
>> +{
>> +   switch (hstride) {
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_0: return BRW_HORIZONTAL_STRIDE_0;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_1: return BRW_HORIZONTAL_STRIDE_1;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_2: return BRW_HORIZONTAL_STRIDE_2;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_4: return BRW_HORIZONTAL_STRIDE_4;
>> +   default:
>> +      unreachable("not reached");
>> +   }
>> +}
>> +
>> +static enum brw_vertical_stride
>> +vstride_from_align1_3src_hstride(enum gen10_align1_3src_src_horizontal_stride hstride)
>> +{
>> +   switch (hstride) {
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_0: return BRW_VERTICAL_STRIDE_0;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_1: return BRW_VERTICAL_STRIDE_1;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_2: return BRW_VERTICAL_STRIDE_2;
>> +   case BRW_ALIGN1_3SRC_SRC_HORIZONTAL_STRIDE_4: return BRW_VERTICAL_STRIDE_4;
>> +   default:
>> +      unreachable("not reached");
>> +   }
>> +}
>> +
>> +/* From "GEN10 Regioning Rules for Align1 Ternary Operations" in the
>> + * "Register Region Restrictions" documentation
>> + */
>> +static enum brw_width
>> +implied_width(enum brw_vertical_stride _vert_stride,
>> +              enum brw_horizontal_stride _horiz_stride)
>> +{
>> +   /* "1. Width is 1 when Vertical and Horizontal Strides are both zero." */
>> +   if (_vert_stride == BRW_VERTICAL_STRIDE_0 &&
>> +       _horiz_stride == BRW_HORIZONTAL_STRIDE_0) {
>> +      return BRW_WIDTH_1;
>> +
>> +   /* "2. Width is equal to vertical stride when Horizontal Stride is zero." */
>> +   } else if (_horiz_stride == BRW_HORIZONTAL_STRIDE_0) {
>> +      switch (_vert_stride) {
>> +      case BRW_VERTICAL_STRIDE_2: return BRW_WIDTH_2;
>> +      case BRW_VERTICAL_STRIDE_4: return BRW_WIDTH_4;
>> +      case BRW_VERTICAL_STRIDE_8: return BRW_WIDTH_8;
>> +      case BRW_VERTICAL_STRIDE_0:
>> +      default:
>> +         unreachable("not reached");
>> +      }
>> +
>> +   } else {
>> +      /* FINISHME: Implement these: */
>
> Maybe just an assert or two would be enough to cover the finishme here.

We don't want the disassembler to ever crash or assert fail. I'm okay
with it printing things slightly wrong for now... since I don't know
what "right" actually is.


More information about the mesa-dev mailing list