[Mesa-dev] [PATCH 2/2] prog_optimize: Add simplify CMP optimization pass

[Jerome Glisse]

On Tue, Apr 5, 2011 at 2:20 AM, Tom Stellard <tstellar at gmail.com> wrote:
> This pass removes conditions from conditional assignments when possible.
> This pass is useful for hardware that requires a lot of lowering passes
> that generate many CMP instructions.
> ---
>  src/mesa/program/prog_optimize.c |   87 ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 87 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/program/prog_optimize.c b/src/mesa/program/prog_optimize.c
> index f62d890..cf579d5 100644
> --- a/src/mesa/program/prog_optimize.c
> +++ b/src/mesa/program/prog_optimize.c
> @@ -1236,6 +1236,92 @@ print_it(struct gl_context *ctx, struct gl_program *program, const char *txt) {
>  }
>  #endif
>
> +/**
> + * This pass removes the condition from conditional assignments by converting
> + * CMP instructions to MOV instructions in the case where the CMP instruction
> + * is the first instruction to write to its destination register.
> + *
> + * There are two scenarios when the condition can be safely removed from a
> + * conditional assignment.  The first is:
> + * int a;
> + *
> + * if (cond)
> + *     a = foo;
> + *
> + * If 'a' is used after this when 'cond' is false, the behavior is
> + * undefined, and if we set 'a' to 'foo' when 'cond' is false, we haven't
> + * changed the behavior of the program.
> + *
> + * The second scenario is:
> + * int a;
> + *
> + * if (cond)
> + *     a = foo;
> + * else
> + *     a = bar;
> + *
> + * This is equivalent to these two conditional assignments:
> + * if (cond) a = foo;
> + * if (!cond) a = bar;
> + *
> + * In this case, it is safe to set 'a' to 'foo' when 'cond' is false, because
> + * 'a' will be overwritten with the value of 'bar' by the second conditional.
> + */
> +static void
> +_mesa_simplify_cmp(struct gl_program * program)
> +{
> +   GLuint tempWrites[REG_ALLOCATE_MAX_PROGRAM_TEMPS];
> +   GLuint outputWrites[MAX_PROGRAM_OUTPUTS];
> +   GLuint i;
> +
> +   if (dbg) {
> +      printf("Optimize: Begin reads without writes\n");
> +      _mesa_print_program(program);
> +   }
> +
> +   for (i = 0; i < REG_ALLOCATE_MAX_PROGRAM_TEMPS; i++) {
> +      tempWrites[i] = 0;
> +   }
> +
> +   for (i = 0; i < MAX_PROGRAM_OUTPUTS; i++) {
> +      outputWrites[i] = 0;
> +   }
> +
> +   for (i = 0; i < program->NumInstructions; i++) {
> +      struct prog_instruction *inst = program->Instructions + i;
> +      GLuint prevWriteMask;
> +
> +      if (_mesa_is_flow_control_opcode(inst->Opcode) || inst->DstReg.RelAddr) {
> +         return;
> +      }
> +
> +      if (inst->DstReg.File == PROGRAM_OUTPUT) {
> +         assert(inst->DstReg.Index < MAX_PROGRAM_OUTPUTS);
> +         prevWriteMask = outputWrites[inst->DstReg.Index];
> +         outputWrites[inst->DstReg.Index] |= inst->DstReg.WriteMask;
> +      } else if (inst->DstReg.File == PROGRAM_TEMPORARY) {
> +         assert(inst->DstReg.Index < REG_ALLOCATE_MAX_PROGRAM_TEMPS);
> +         prevWriteMask = tempWrites[inst->DstReg.Index];
> +         tempWrites[inst->DstReg.Index] |= inst->DstReg.WriteMask;
> +      }
> +
> +      /* For a CMP to be considered a conditional write, the destination
> +       * register and source register two must be the same. */
> +      if (inst->Opcode == OPCODE_CMP
> +          && !(inst->DstReg.WriteMask & prevWriteMask)
> +          && inst->SrcReg[2].File == inst->DstReg.File
> +          && inst->SrcReg[2].Index == inst->DstReg.Index
> +          && inst->DstReg.WriteMask == get_src_arg_mask(inst, 2, NO_MASK)) {
> +
> +         inst->Opcode = OPCODE_MOV;
> +         inst->SrcReg[0] = inst->SrcReg[1];
> +      }
> +   }
> +   if (dbg) {
> +      printf("Optimize: End reads without writes\n");
> +      _mesa_print_program(program);
> +   }
> +}
>
>  /**
>  * Apply optimizations to the given program to eliminate unnecessary
> @@ -1246,6 +1332,7 @@ _mesa_optimize_program(struct gl_context *ctx, struct gl_program *program)
>  {
>    GLboolean any_change;
>
> +   _mesa_simplify_cmp(program);
>    /* Stop when no modifications were output */
>    do {
>       any_change = GL_FALSE;
> --
> 1.7.3.4
>

The comment isn't very accurate on what the code does (comment is
about GLSL construct not ARB shader asm one). I think a comment
explaining what pattern the code replace would be helpful. If i am not
mistaken :
CMP A, x, y, A replaced MOV A, y
when A was not previoulsy written/modified in any previous instruction.

Otherwise this look good as it bail if there is control flow stuff
(maybe adding a comment to make sure no one remove the control flow
test).

Cheers,
Jerome