[Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types
The hardware encodings often mean different things depending on whether the source is an immediate. --- src/intel/compiler/brw_disasm.c | 46 --- src/intel/compiler/brw_eu_compact.c | 8 +-- src/intel/compiler/brw_eu_defines.h | 48 +-- src/intel/compiler/brw_eu_emit.c | 109 +-- src/intel/compiler/brw_eu_validate.c | 60 +-- src/intel/compiler/brw_reg.h | 2 + 6 files changed, 144 insertions(+), 129 deletions(-) diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c index 3a33614523..b5c283058a 100644 --- a/src/intel/compiler/brw_disasm.c +++ b/src/intel/compiler/brw_disasm.c @@ -238,17 +238,18 @@ static const char *const access_mode[2] = { }; static const char * const reg_encoding[] = { - [BRW_HW_REG_TYPE_UD] = "UD", - [BRW_HW_REG_TYPE_D] = "D", - [BRW_HW_REG_TYPE_UW] = "UW", - [BRW_HW_REG_TYPE_W] = "W", - [BRW_HW_REG_NON_IMM_TYPE_UB] = "UB", - [BRW_HW_REG_NON_IMM_TYPE_B] = "B", - [GEN7_HW_REG_NON_IMM_TYPE_DF] = "DF", - [BRW_HW_REG_TYPE_F] = "F", - [GEN8_HW_REG_TYPE_UQ] = "UQ", - [GEN8_HW_REG_TYPE_Q] = "Q", - [GEN8_HW_REG_NON_IMM_TYPE_HF] = "HF", + [BRW_HW_REG_TYPE_UD] = "UD", + [BRW_HW_REG_TYPE_D] = "D", + [BRW_HW_REG_TYPE_UW] = "UW", + [BRW_HW_REG_TYPE_W] = "W", + [BRW_HW_REG_TYPE_F] = "F", + [GEN8_HW_REG_TYPE_UQ] = "UQ", + [GEN8_HW_REG_TYPE_Q] = "Q", + + [BRW_HW_REG_TYPE_UB] = "UB", + [BRW_HW_REG_TYPE_B] = "B", + [GEN7_HW_REG_TYPE_DF] = "DF", + [GEN8_HW_REG_TYPE_HF] = "HF", }; static const char *const three_source_reg_encoding[] = { @@ -1024,41 +1025,42 @@ src2_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst *ins } static int -imm(FILE *file, const struct gen_device_info *devinfo, unsigned type, const brw_inst *inst) +imm(FILE *file, const struct gen_device_info *devinfo, enum hw_imm_type type, +const brw_inst *inst) { switch (type) { - case BRW_HW_REG_TYPE_UD: + case BRW_HW_IMM_TYPE_UD: format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); break; - case BRW_HW_REG_TYPE_D: + case BRW_HW_IMM_TYPE_D: format(file, "%dD", brw_inst_imm_d(devinfo, inst)); break; - case BRW_HW_REG_TYPE_UW: + case BRW_HW_IMM_TYPE_UW: format(file, "0x%04xUW", (uint16_t) brw_inst_imm_ud(devinfo, inst)); break; - case BRW_HW_REG_TYPE_W: + case BRW_HW_IMM_TYPE_W: format(file, "%dW", (int16_t) brw_inst_imm_d(devinfo, inst)); break; - case BRW_HW_REG_IMM_TYPE_UV: + case BRW_HW_IMM_TYPE_UV: format(file, "0x%08xUV", brw_inst_imm_ud(devinfo, inst)); break; - case BRW_HW_REG_IMM_TYPE_VF: + case BRW_HW_IMM_TYPE_VF: format(file, "[%-gF, %-gF, %-gF, %-gF]VF", brw_vf_to_float(brw_inst_imm_ud(devinfo, inst)), brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 8), brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 16), brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 24)); break; - case BRW_HW_REG_IMM_TYPE_V: + case BRW_HW_IMM_TYPE_V: format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); break; - case BRW_HW_REG_TYPE_F: + case BRW_HW_IMM_TYPE_F: format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); break; - case GEN8_HW_REG_IMM_TYPE_DF: + case GEN8_HW_IMM_TYPE_DF: format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); break; - case GEN8_HW_REG_IMM_TYPE_HF: + case GEN8_HW_IMM_TYPE_HF: string(file, "Half Float IMM"); break; } diff --git a/src/intel/compiler/brw_eu_compact.c b/src/intel/compiler/brw_eu_compact.c index 79103d7883..bca526f592 100644 --- a/src/intel/compiler/brw_eu_compact.c +++ b/src/intel/compiler/brw_eu_compact.c @@ -995,9 +995,9 @@ precompact(const struct gen_device_info *devinfo, brw_inst inst) !(devinfo->is_haswell && brw_inst_opcode(devinfo, &inst) == BRW_OPCODE_DIM) && !(devinfo->gen >= 8 && - (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_IMM_TYPE_DF || - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_UQ || - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_Q))) { + (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_DF || + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_UQ || + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_Q))) { brw_inst_set_src1_reg_type(devinfo, &inst, BRW_HW_REG_TYPE_UD); } @@ -1016,7 +1016,7 @@ precompact(const struct gen_device_info *devinfo, brw_inst inst) brw_inst_src0_reg_type(devinfo, &inst) == BRW_HW_REG_TYPE_F && brw_inst_dst_reg_type(devinfo, &inst) == BRW_HW_REG_TYPE_F && brw_inst_dst_hstride(devinfo, &inst) == BRW_HORIZONTAL_STRIDE_1) { - brw_inst_set_src0_reg_type(devinfo, &ins
Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types
Matt Turner writes: > The hardware encodings often mean different things depending on whether > the source is an immediate. > --- > src/intel/compiler/brw_disasm.c | 46 --- > src/intel/compiler/brw_eu_compact.c | 8 +-- > src/intel/compiler/brw_eu_defines.h | 48 +-- > src/intel/compiler/brw_eu_emit.c | 109 > +-- > src/intel/compiler/brw_eu_validate.c | 60 +-- > src/intel/compiler/brw_reg.h | 2 + > 6 files changed, 144 insertions(+), 129 deletions(-) > > diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c > index 3a33614523..b5c283058a 100644 > --- a/src/intel/compiler/brw_disasm.c > +++ b/src/intel/compiler/brw_disasm.c > @@ -238,17 +238,18 @@ static const char *const access_mode[2] = { > }; > > static const char * const reg_encoding[] = { > - [BRW_HW_REG_TYPE_UD] = "UD", > - [BRW_HW_REG_TYPE_D] = "D", > - [BRW_HW_REG_TYPE_UW] = "UW", > - [BRW_HW_REG_TYPE_W] = "W", > - [BRW_HW_REG_NON_IMM_TYPE_UB] = "UB", > - [BRW_HW_REG_NON_IMM_TYPE_B] = "B", > - [GEN7_HW_REG_NON_IMM_TYPE_DF] = "DF", > - [BRW_HW_REG_TYPE_F] = "F", > - [GEN8_HW_REG_TYPE_UQ] = "UQ", > - [GEN8_HW_REG_TYPE_Q] = "Q", > - [GEN8_HW_REG_NON_IMM_TYPE_HF] = "HF", > + [BRW_HW_REG_TYPE_UD] = "UD", > + [BRW_HW_REG_TYPE_D] = "D", > + [BRW_HW_REG_TYPE_UW] = "UW", > + [BRW_HW_REG_TYPE_W] = "W", > + [BRW_HW_REG_TYPE_F] = "F", > + [GEN8_HW_REG_TYPE_UQ] = "UQ", > + [GEN8_HW_REG_TYPE_Q] = "Q", > + > + [BRW_HW_REG_TYPE_UB] = "UB", > + [BRW_HW_REG_TYPE_B] = "B", > + [GEN7_HW_REG_TYPE_DF] = "DF", > + [GEN8_HW_REG_TYPE_HF] = "HF", > }; > > static const char *const three_source_reg_encoding[] = { > @@ -1024,41 +1025,42 @@ src2_3src(FILE *file, const struct gen_device_info > *devinfo, const brw_inst *ins > } > > static int > -imm(FILE *file, const struct gen_device_info *devinfo, unsigned type, const > brw_inst *inst) > +imm(FILE *file, const struct gen_device_info *devinfo, enum hw_imm_type type, > +const brw_inst *inst) > { > switch (type) { > - case BRW_HW_REG_TYPE_UD: > + case BRW_HW_IMM_TYPE_UD: >format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_D: > + case BRW_HW_IMM_TYPE_D: >format(file, "%dD", brw_inst_imm_d(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_UW: > + case BRW_HW_IMM_TYPE_UW: >format(file, "0x%04xUW", (uint16_t) brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_W: > + case BRW_HW_IMM_TYPE_W: >format(file, "%dW", (int16_t) brw_inst_imm_d(devinfo, inst)); >break; > - case BRW_HW_REG_IMM_TYPE_UV: > + case BRW_HW_IMM_TYPE_UV: >format(file, "0x%08xUV", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_IMM_TYPE_VF: > + case BRW_HW_IMM_TYPE_VF: >format(file, "[%-gF, %-gF, %-gF, %-gF]VF", > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst)), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 8), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 16), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 24)); >break; > - case BRW_HW_REG_IMM_TYPE_V: > + case BRW_HW_IMM_TYPE_V: >format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_F: > + case BRW_HW_IMM_TYPE_F: >format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); >break; > - case GEN8_HW_REG_IMM_TYPE_DF: > + case GEN8_HW_IMM_TYPE_DF: >format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); >break; > - case GEN8_HW_REG_IMM_TYPE_HF: > + case GEN8_HW_IMM_TYPE_HF: >string(file, "Half Float IMM"); >break; > } > diff --git a/src/intel/compiler/brw_eu_compact.c > b/src/intel/compiler/brw_eu_compact.c > index 79103d7883..bca526f592 100644 > --- a/src/intel/compiler/brw_eu_compact.c > +++ b/src/intel/compiler/brw_eu_compact.c > @@ -995,9 +995,9 @@ precompact(const struct gen_device_info *devinfo, > brw_inst inst) > !(devinfo->is_haswell && > brw_inst_opcode(devinfo, &inst) == BRW_OPCODE_DIM) && > !(devinfo->gen >= 8 && > - (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_IMM_TYPE_DF > || > - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_UQ || > - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_Q))) { > + (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_DF || > + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_UQ || > + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_Q))) { >brw_inst_set_src1_reg_type(devinfo, &inst, BRW_HW_REG_TYPE_UD); > } > > @@ -1016,7 +1016,7 @@ precompact(const struct gen_device_info *devinfo, > brw_ins
Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types
On Tue, Aug 8, 2017 at 4:25 PM, Scott D Phillips wrote: >> + [BRW_HW_IMM_TYPE_UV] = 2, >> + [BRW_HW_IMM_TYPE_VF] = 4, >> + [BRW_HW_IMM_TYPE_V] = 2, > > Is this right? I see it was there before, and perhaps I'm being dense, > but it seems like V and UV should be size 4 from the PRM. Yes. The encoded immediates themselves are 4 bytes, but this table captures the size of the individual components once expanded. That's admittedly a little weird. A V/UV immediate consists of 8x 4-bit integer values. A restriction documented in "Gen Graphics » BSpec » 3D and Compute Engine » 3D and GPGPU Programs » EU Overview » Registers and Register Regions » Immediate" states "When an immediate vector is used in an instruction, the destination must be 128-bit aligned with destination horizontal stride equivalent to a word for an immediate integer vector (v) and equivalent to a DWord for an immediate float vector (vf).". So we consider the individual components of the V/UV immediate to really be words, of size 2. Thanks for the good question! ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types
Matt Turner writes: > On Tue, Aug 8, 2017 at 4:25 PM, Scott D Phillips > wrote: >>> + [BRW_HW_IMM_TYPE_UV] = 2, >>> + [BRW_HW_IMM_TYPE_VF] = 4, >>> + [BRW_HW_IMM_TYPE_V] = 2, >> >> Is this right? I see it was there before, and perhaps I'm being dense, >> but it seems like V and UV should be size 4 from the PRM. > > Yes. The encoded immediates themselves are 4 bytes, but this table > captures the size of the individual components once expanded. That's > admittedly a little weird. > > A V/UV immediate consists of 8x 4-bit integer values. A restriction > documented in "Gen Graphics » BSpec » 3D and Compute Engine » 3D and > GPGPU Programs » EU Overview » Registers and Register Regions » > Immediate" states "When an immediate vector is used in an instruction, > the destination must be 128-bit aligned with destination horizontal > stride equivalent to a word for an immediate integer vector (v) and > equivalent to a DWord for an immediate float vector (vf).". > > So we consider the individual components of the V/UV immediate to > really be words, of size 2. > > Thanks for the good question! Ah, I see. Thanks for the explanation. Reviewed-by: Scott D Phillips ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev