Re: [Mesa-dev] [PATCH 03/13] i965: Print subreg in units of type-size on ternary instructions

[Matt Turner]

On Fri, Sep 29, 2017 at 5:03 PM, Scott D Phillips
 wrote:
> Matt Turner  writes:
>
>> The instruction word contains SubRegNum[4:2] so it's in units of dwords
>> (hence the * 4 to get it in terms of bytes). Before this patch, the
>> subreg would have been wrong for DF arguments.
>
> Trying to grok the subregnum stuff here, in brw_eu_emit.c I see:
>
> static int
> get_3src_subreg_nr(struct brw_reg reg)
> {
>
>/* Normally, SubRegNum is in bytes (0..31).  However, 3-src
> * instructions use 32-bit units (components 0..7).  Since they
> * only support F/D/UD types, this doesn't lose any
> * flexibility, but uses fewer bits.
> */
>
>return reg.subnr / 4;
> }
>
> Does the code above need updated for DF registers too? Or is that
> already accounted for somehow?

Nope, it does not, but that's a good question nonetheless.

Align1 mode has SubRegNum[4:0], which means it can represent
subregister numbers from 0-31. Regular (1 and 2 source) align16
instructions only have SubRegNum[4], which means it can represent
subregister numbers of only 0 and 16.

3-source instructions, which until CNL are only available in align16
mode, use a different instruction format in order to fit the extra
source. In order to do that, they lose some flexibility available to
1-2 source instructions like the full range of datatypes. Only F, D,
UD, and DF datatypes are supported, so there's no need for subregister
bits for offsets lower than four bytes, but they do offer a bit for
replication (called RepCtrl) that picks a single value and replicates
it to all channels. For that you want more than just the SubRegNum[4]
offered by regular align16 instructions, so they made it
SubRegNum[4:2], which is capable of representing any four-byte aligned
subregister.

With doubles, you'd just never use bit 2.

The code in get_3src_subreg_nr() is just an easy hack chop off the low
two bits from reg.subnr in order to go to the SubRegNum[4:2] form.

> I'm probably just missing something. With an explanation Patches 1-3 are:
>
> Reviewed-by: Scott D Phillips 

Thanks!
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 03/13] i965: Print subreg in units of type-size on ternary instructions

[Scott D Phillips]

Matt Turner  writes:

> The instruction word contains SubRegNum[4:2] so it's in units of dwords
> (hence the * 4 to get it in terms of bytes). Before this patch, the
> subreg would have been wrong for DF arguments.

Trying to grok the subregnum stuff here, in brw_eu_emit.c I see:

static int
get_3src_subreg_nr(struct brw_reg reg)
{

   /* Normally, SubRegNum is in bytes (0..31).  However, 3-src
* instructions use 32-bit units (components 0..7).  Since they
* only support F/D/UD types, this doesn't lose any
* flexibility, but uses fewer bits.
*/

   return reg.subnr / 4;
}

Does the code above need updated for DF registers too? Or is that
already accounted for somehow?

I'm probably just missing something. With an explanation Patches 1-3 are:

Reviewed-by: Scott D Phillips 

> ---
>  src/intel/compiler/brw_disasm.c | 31 ++-
>  1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
> index e2675b5f4c..188c7c53d0 100644
> --- a/src/intel/compiler/brw_disasm.c
> +++ b/src/intel/compiler/brw_disasm.c
> @@ -764,6 +764,12 @@ dest_3src(FILE *file, const struct gen_device_info 
> *devinfo, const brw_inst *ins
>  {
> int err = 0;
> uint32_t reg_file;
> +   enum brw_reg_type type =
> +  brw_hw_3src_type_to_reg_type(devinfo,
> +   brw_inst_3src_dst_type(devinfo, inst));
> +   unsigned dst_subreg_nr =
> +  brw_inst_3src_dst_subreg_nr(devinfo, inst) * 4 /
> +  brw_reg_type_to_size(type);
>  
> if (devinfo->gen == 6 && brw_inst_3src_dst_reg_file(devinfo, inst))
>reg_file = BRW_MESSAGE_REGISTER_FILE;
> @@ -773,8 +779,8 @@ 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 (brw_inst_3src_dst_subreg_nr(devinfo, inst))
> -  format(file, ".%"PRIu64, brw_inst_3src_dst_subreg_nr(devinfo, inst));
> +   if (dst_subreg_nr)
> +  format(file, ".%u", dst_subreg_nr);
> string(file, "<1>");
> err |= control(file, "writemask", writemask,
>brw_inst_3src_dst_writemask(devinfo, inst), NULL);
> @@ -928,7 +934,12 @@ static int
>  src0_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
> *inst)
>  {
> int err = 0;
> -   unsigned src0_subreg_nr = brw_inst_3src_src0_subreg_nr(devinfo, inst);
> +   enum brw_reg_type type =
> +  brw_hw_3src_type_to_reg_type(devinfo,
> +   brw_inst_3src_src_type(devinfo, inst));
> +   unsigned src0_subreg_nr =
> +  brw_inst_3src_src0_subreg_nr(devinfo, inst) * 4 /
> +  brw_reg_type_to_size(type);
>  
> err |= control(file, "negate", m_negate,
>brw_inst_3src_src0_negate(devinfo, inst), NULL);
> @@ -955,7 +966,12 @@ static int
>  src1_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
> *inst)
>  {
> int err = 0;
> -   unsigned src1_subreg_nr = brw_inst_3src_src1_subreg_nr(devinfo, inst);
> +   enum brw_reg_type type =
> +  brw_hw_3src_type_to_reg_type(devinfo,
> +   brw_inst_3src_src_type(devinfo, inst));
> +   unsigned src1_subreg_nr =
> +  brw_inst_3src_src1_subreg_nr(devinfo, inst) * 4 /
> +  brw_reg_type_to_size(type);
>  
> err |= control(file, "negate", m_negate,
>brw_inst_3src_src1_negate(devinfo, inst), NULL);
> @@ -983,7 +999,12 @@ static int
>  src2_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
> *inst)
>  {
> int err = 0;
> -   unsigned src2_subreg_nr = brw_inst_3src_src2_subreg_nr(devinfo, inst);
> +   enum brw_reg_type type =
> +  brw_hw_3src_type_to_reg_type(devinfo,
> +   brw_inst_3src_src_type(devinfo, inst));
> +   unsigned src2_subreg_nr =
> +  brw_inst_3src_src2_subreg_nr(devinfo, inst) * 4 /
> +  brw_reg_type_to_size(type);
>  
> err |= control(file, "negate", m_negate,
>brw_inst_3src_src2_negate(devinfo, inst), NULL);
> -- 
> 2.13.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 03/13] i965: Print subreg in units of type-size on ternary instructions

[Matt Turner]

The instruction word contains SubRegNum[4:2] so it's in units of dwords
(hence the * 4 to get it in terms of bytes). Before this patch, the
subreg would have been wrong for DF arguments.
---
 src/intel/compiler/brw_disasm.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c
index e2675b5f4c..188c7c53d0 100644
--- a/src/intel/compiler/brw_disasm.c
+++ b/src/intel/compiler/brw_disasm.c
@@ -764,6 +764,12 @@ dest_3src(FILE *file, const struct gen_device_info 
*devinfo, const brw_inst *ins
 {
int err = 0;
uint32_t reg_file;
+   enum brw_reg_type type =
+  brw_hw_3src_type_to_reg_type(devinfo,
+   brw_inst_3src_dst_type(devinfo, inst));
+   unsigned dst_subreg_nr =
+  brw_inst_3src_dst_subreg_nr(devinfo, inst) * 4 /
+  brw_reg_type_to_size(type);
 
if (devinfo->gen == 6 && brw_inst_3src_dst_reg_file(devinfo, inst))
   reg_file = BRW_MESSAGE_REGISTER_FILE;
@@ -773,8 +779,8 @@ 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 (brw_inst_3src_dst_subreg_nr(devinfo, inst))
-  format(file, ".%"PRIu64, brw_inst_3src_dst_subreg_nr(devinfo, inst));
+   if (dst_subreg_nr)
+  format(file, ".%u", dst_subreg_nr);
string(file, "<1>");
err |= control(file, "writemask", writemask,
   brw_inst_3src_dst_writemask(devinfo, inst), NULL);
@@ -928,7 +934,12 @@ static int
 src0_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
*inst)
 {
int err = 0;
-   unsigned src0_subreg_nr = brw_inst_3src_src0_subreg_nr(devinfo, inst);
+   enum brw_reg_type type =
+  brw_hw_3src_type_to_reg_type(devinfo,
+   brw_inst_3src_src_type(devinfo, inst));
+   unsigned src0_subreg_nr =
+  brw_inst_3src_src0_subreg_nr(devinfo, inst) * 4 /
+  brw_reg_type_to_size(type);
 
err |= control(file, "negate", m_negate,
   brw_inst_3src_src0_negate(devinfo, inst), NULL);
@@ -955,7 +966,12 @@ static int
 src1_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
*inst)
 {
int err = 0;
-   unsigned src1_subreg_nr = brw_inst_3src_src1_subreg_nr(devinfo, inst);
+   enum brw_reg_type type =
+  brw_hw_3src_type_to_reg_type(devinfo,
+   brw_inst_3src_src_type(devinfo, inst));
+   unsigned src1_subreg_nr =
+  brw_inst_3src_src1_subreg_nr(devinfo, inst) * 4 /
+  brw_reg_type_to_size(type);
 
err |= control(file, "negate", m_negate,
   brw_inst_3src_src1_negate(devinfo, inst), NULL);
@@ -983,7 +999,12 @@ static int
 src2_3src(FILE *file, const struct gen_device_info *devinfo, const brw_inst 
*inst)
 {
int err = 0;
-   unsigned src2_subreg_nr = brw_inst_3src_src2_subreg_nr(devinfo, inst);
+   enum brw_reg_type type =
+  brw_hw_3src_type_to_reg_type(devinfo,
+   brw_inst_3src_src_type(devinfo, inst));
+   unsigned src2_subreg_nr =
+  brw_inst_3src_src2_subreg_nr(devinfo, inst) * 4 /
+  brw_reg_type_to_size(type);
 
err |= control(file, "negate", m_negate,
   brw_inst_3src_src2_negate(devinfo, inst), NULL);
-- 
2.13.5

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev