Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types

[Scott D Phillips]

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

Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types

[Matt Turner]

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

[Scott D Phillips]

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, ) == BRW_OPCODE_DIM) &&
> !(devinfo->gen >= 8 &&
> - (brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_REG_IMM_TYPE_DF 
> ||
> -  brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_REG_TYPE_UQ ||
> -  brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_REG_TYPE_Q))) {
> + (brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_IMM_TYPE_DF ||
> +  brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_IMM_TYPE_UQ ||
> +  brw_inst_src0_reg_type(devinfo, ) == GEN8_HW_IMM_TYPE_Q))) {
>brw_inst_set_src1_reg_type(devinfo, , BRW_HW_REG_TYPE_UD);
> }
>  
> @@ -1016,7 +1016,7 @@ precompact(const struct gen_device_info *devinfo, 
> brw_inst inst)
>