Re: [Mesa-dev] [PATCH 1/4] i965: Rework opt_vector_float() control flow.

[Matt Turner]

On Sun, Apr 17, 2016 at 11:14 PM, Kenneth Graunke  wrote:
> This reworks opt_vector_float() so that there's only one place that
> flushes out any accumulated state and emits a VF.
>
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 63 
> +++---
>  1 file changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 6433fc5..fa0d80d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -385,48 +385,55 @@ vec4_visitor::opt_vector_float()
> unsigned writemask = 0;
>
> foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
> -  if (last_reg != inst->dst.nr ||
> +  int vf = -1;
> +
> +  /* Look for unconditional MOVs from an immediate with a partial
> +   * writemask.  See if the immediate can be represented as a VF.
> +   */
> +  if (inst->opcode == BRW_OPCODE_MOV &&
> +  inst->src[0].file == IMM &&
> +  inst->predicate == BRW_PREDICATE_NONE &&
> +  inst->dst.writemask != WRITEMASK_XYZW) {
> + vf = brw_float_to_vf(inst->src[0].f);
> +  }
> +
> +  /* If this wasn't a MOV, or the value was non-representable, or
> +   * the destination register doesn't match, then this breaks our
> +   * sequence.  Combine anything we've accumulated so far.
> +   */
> +  if (vf == -1 ||

Is the value being non-representable an important point? Running
shader-db on the first three patches shows that something hurt 20
programs, and I think it's considering a non-representable value to
break the sequence.

For instance, in metro-last-light/2175 we see:

-mov(8)  g16<1>.yD   1033476506D
-mov(8)  g16<1>.xzF  [0.375F, 0F, 2.5F, 0F]VF
+mov(8)  g16<1>.xD   1052770304D
+mov(8)  g16<1>.yD   1033476506D
+mov(8)  g16<1>.zD   1075838976D

where the .y component isn't representable but x and z are. I suspect
the other 19 cases are the same problem.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/4] i965: Rework opt_vector_float() control flow.

[Iago Toral]

Patches 1-3 are:

Reviewed-by: Iago Toral Quiroga 

On Sun, 2016-04-17 at 23:14 -0700, Kenneth Graunke wrote:
> This reworks opt_vector_float() so that there's only one place that
> flushes out any accumulated state and emits a VF.
> 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 63 
> +++---
>  1 file changed, 35 insertions(+), 28 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 6433fc5..fa0d80d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -385,48 +385,55 @@ vec4_visitor::opt_vector_float()
> unsigned writemask = 0;
>  
> foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
> -  if (last_reg != inst->dst.nr ||
> +  int vf = -1;
> +
> +  /* Look for unconditional MOVs from an immediate with a partial
> +   * writemask.  See if the immediate can be represented as a VF.
> +   */
> +  if (inst->opcode == BRW_OPCODE_MOV &&
> +  inst->src[0].file == IMM &&
> +  inst->predicate == BRW_PREDICATE_NONE &&
> +  inst->dst.writemask != WRITEMASK_XYZW) {
> + vf = brw_float_to_vf(inst->src[0].f);
> +  }
> +
> +  /* If this wasn't a MOV, or the value was non-representable, or
> +   * the destination register doesn't match, then this breaks our
> +   * sequence.  Combine anything we've accumulated so far.
> +   */
> +  if (vf == -1 ||
> +  last_reg != inst->dst.nr ||
>last_reg_offset != inst->dst.reg_offset ||
>last_reg_file != inst->dst.file) {
>   progress |= vectorize_mov(block, inst, imm, imm_inst, inst_count,
> writemask);
>   inst_count = 0;
> + last_reg = -1;
>   writemask = 0;
> - last_reg = inst->dst.nr;
> - last_reg_offset = inst->dst.reg_offset;
> - last_reg_file = inst->dst.file;
>  
>   for (int i = 0; i < 4; i++) {
>  imm[i] = 0;
>   }
>}
>  
> -  if (inst->opcode != BRW_OPCODE_MOV ||
> -  inst->dst.writemask == WRITEMASK_XYZW ||
> -  inst->src[0].file != IMM ||
> -  inst->predicate != BRW_PREDICATE_NONE) {
> - progress |= vectorize_mov(block, inst, imm, imm_inst, inst_count,
> -   writemask);
> - inst_count = 0;
> - last_reg = -1;
> - continue;
> -  }
> +  /* Record this instruction's value (if it was representable). */
> +  if (vf != -1) {
> + if ((inst->dst.writemask & WRITEMASK_X) != 0)
> +imm[0] = vf;
> + if ((inst->dst.writemask & WRITEMASK_Y) != 0)
> +imm[1] = vf;
> + if ((inst->dst.writemask & WRITEMASK_Z) != 0)
> +imm[2] = vf;
> + if ((inst->dst.writemask & WRITEMASK_W) != 0)
> +imm[3] = vf;
>  
> -  int vf = brw_float_to_vf(inst->src[0].f);
> -  if (vf == -1)
> - continue;
> + writemask |= inst->dst.writemask;
> + imm_inst[inst_count++] = inst;
>  
> -  if ((inst->dst.writemask & WRITEMASK_X) != 0)
> - imm[0] = vf;
> -  if ((inst->dst.writemask & WRITEMASK_Y) != 0)
> - imm[1] = vf;
> -  if ((inst->dst.writemask & WRITEMASK_Z) != 0)
> - imm[2] = vf;
> -  if ((inst->dst.writemask & WRITEMASK_W) != 0)
> - imm[3] = vf;
> -
> -  writemask |= inst->dst.writemask;
> -  imm_inst[inst_count++] = inst;
> + last_reg = inst->dst.nr;
> + last_reg_offset = inst->dst.reg_offset;
> + last_reg_file = inst->dst.file;
> +  }
> }
>  
> if (progress)


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

[Mesa-dev] [PATCH 1/4] i965: Rework opt_vector_float() control flow.

[Kenneth Graunke]

This reworks opt_vector_float() so that there's only one place that
flushes out any accumulated state and emits a VF.

Signed-off-by: Kenneth Graunke 
---
 src/mesa/drivers/dri/i965/brw_vec4.cpp | 63 +++---
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 6433fc5..fa0d80d 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -385,48 +385,55 @@ vec4_visitor::opt_vector_float()
unsigned writemask = 0;
 
foreach_block_and_inst_safe(block, vec4_instruction, inst, cfg) {
-  if (last_reg != inst->dst.nr ||
+  int vf = -1;
+
+  /* Look for unconditional MOVs from an immediate with a partial
+   * writemask.  See if the immediate can be represented as a VF.
+   */
+  if (inst->opcode == BRW_OPCODE_MOV &&
+  inst->src[0].file == IMM &&
+  inst->predicate == BRW_PREDICATE_NONE &&
+  inst->dst.writemask != WRITEMASK_XYZW) {
+ vf = brw_float_to_vf(inst->src[0].f);
+  }
+
+  /* If this wasn't a MOV, or the value was non-representable, or
+   * the destination register doesn't match, then this breaks our
+   * sequence.  Combine anything we've accumulated so far.
+   */
+  if (vf == -1 ||
+  last_reg != inst->dst.nr ||
   last_reg_offset != inst->dst.reg_offset ||
   last_reg_file != inst->dst.file) {
  progress |= vectorize_mov(block, inst, imm, imm_inst, inst_count,
writemask);
  inst_count = 0;
+ last_reg = -1;
  writemask = 0;
- last_reg = inst->dst.nr;
- last_reg_offset = inst->dst.reg_offset;
- last_reg_file = inst->dst.file;
 
  for (int i = 0; i < 4; i++) {
 imm[i] = 0;
  }
   }
 
-  if (inst->opcode != BRW_OPCODE_MOV ||
-  inst->dst.writemask == WRITEMASK_XYZW ||
-  inst->src[0].file != IMM ||
-  inst->predicate != BRW_PREDICATE_NONE) {
- progress |= vectorize_mov(block, inst, imm, imm_inst, inst_count,
-   writemask);
- inst_count = 0;
- last_reg = -1;
- continue;
-  }
+  /* Record this instruction's value (if it was representable). */
+  if (vf != -1) {
+ if ((inst->dst.writemask & WRITEMASK_X) != 0)
+imm[0] = vf;
+ if ((inst->dst.writemask & WRITEMASK_Y) != 0)
+imm[1] = vf;
+ if ((inst->dst.writemask & WRITEMASK_Z) != 0)
+imm[2] = vf;
+ if ((inst->dst.writemask & WRITEMASK_W) != 0)
+imm[3] = vf;
 
-  int vf = brw_float_to_vf(inst->src[0].f);
-  if (vf == -1)
- continue;
+ writemask |= inst->dst.writemask;
+ imm_inst[inst_count++] = inst;
 
-  if ((inst->dst.writemask & WRITEMASK_X) != 0)
- imm[0] = vf;
-  if ((inst->dst.writemask & WRITEMASK_Y) != 0)
- imm[1] = vf;
-  if ((inst->dst.writemask & WRITEMASK_Z) != 0)
- imm[2] = vf;
-  if ((inst->dst.writemask & WRITEMASK_W) != 0)
- imm[3] = vf;
-
-  writemask |= inst->dst.writemask;
-  imm_inst[inst_count++] = inst;
+ last_reg = inst->dst.nr;
+ last_reg_offset = inst->dst.reg_offset;
+ last_reg_file = inst->dst.file;
+  }
}
 
if (progress)
-- 
2.8.0

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