[Mesa-dev] [PATCH 3/3] intel/compiler: add an optimization pass for booleans

[Iago Toral]

On Tue, 2018-07-03 at 18:45 -0700, Caio Marcelo de Oliveira Filho
wrote:
> Hi,
> 
> 
> > +   /* Look for any follow-up instructions that sources from the
> > boolean
> > +    * result of the producer instruction and rewrite them to use
> > the correct
> > +    * bit-size.
> > +    */
> > +   foreach_inst_in_block_starting_from(fs_inst, fixup_inst, inst)
> > {
> > +      if (!inst_supports_boolean(fixup_inst))
> > +         continue;
> > +
> > +      /* For MOV instructions we can always rewrite the boolean
> > source
> > +       * if the instrucion reads the same region we produced in
> > the
> > +       * 32-bit conversion.
> > +       */
> > +      if (fixup_inst->opcode == BRW_OPCODE_MOV &&
> > +          region_match(inst->dst, inst->size_written,
> > +                       fixup_inst->src[0], fixup_inst-
> > >size_read(0))) {
> > +         if (propagate_from_source) {
> > +            fixup_inst->src[0].file = inst->src[0].file;
> > +            fixup_inst->src[0].nr = inst->src[0].nr;
> > +         }
> > +         fixup_inst->src[0] =
> > +            fix_bool_reg_bit_size(fixup_inst->src[0], bit_size);
> > +         progress = true;
> > +         continue;
> > +      }
> 
> It seems the rest of the code assumes that instruction is not MOV, so
> you would need to ensure continue is called regardless the region
> match.

Right, although if the region doesn't match the rest of the code won't
do anything anyway.

> Idea: it seems we could just remove this section above (handling
> MOV),
> and slightly change the section below so that MOV can be dealt with
> it
> too.
> 
> - Drop the section above;
> - Rename progress_logical to local_progress;
> - Add a "fixup_inst->opcode == BRW_OPCODE_MOV" to the

The recursive call executes for logical instructions, not for MOV, so
this should be !=.

>   condition that controls the recursive call;
> - Update comments accordingly.

Sounds like a good idea, thanks for the feedback.

Iago

> 
> > +
> > +      /* For logical instructions we have the same restriction as
> > for MOVs,
> > +       * and we also need to:
> > +       *
> > +       * 1. Propagate the bit-size to the boolean destination of
> > the
> > +       *    instruction.
> > +       * 2. Rewrite any instruction that reads the destination to
> > use
> > +       *    the new bit-size.
> > +       *
> > +       * However, we can only do these if we can rewrite all the
> > operands
> > +       * to use the same bit-size.
> > +       */
> > +      bool progress_logical = false;
> > +      bool same_bit_size = true;
> > +      for (unsigned i = 0; i < fixup_inst->sources; i++) {
> > +         if (region_match(inst->dst, inst->size_written,
> > +                          fixup_inst->src[i], fixup_inst-
> > >size_read(i))) {
> > +            if (propagate_from_source) {
> > +               fixup_inst->src[i].file = inst->src[0].file;
> > +               fixup_inst->src[i].nr = inst->src[0].nr;
> > +            }
> > +            fixup_inst->src[i] =
> > +               fix_bool_reg_bit_size(fixup_inst->src[i],
> > bit_size);
> > +            progress_logical = true;
> > +            progress = true;
> > +         }
> > +
> > +         if (i > 0 &&
> > +             type_sz(fixup_inst->src[i].type) !=
> > +             type_sz(fixup_inst->src[i - 1].type)) {
> > +            same_bit_size = false;
> > +         }
> > +      }
> > +
> > +      /* If we have successfully rewritten a logical instruction
> > operand
> > +       * to use a smaller bit-size boolean and all the operands in
> > the
> > +       * instruction have the same small bit-size, then propagate
> > the
> > +       * new bit-size to the destination boolean and do the same
> > for all
> > +       * follow-up instructions that read from it.
> > +       */
> > +      if (progress_logical && same_bit_size) {
> > +         fixup_inst->dst = retype(fixup_inst->dst, fixup_inst-
> > >src[0].type);
> > +         propagate_bool_bit_size(fixup_inst, false);
> > +      }
> > +   }
> > +
> > +   return progress;
> > +}
> 
> 
> 
> 
> Thanks,
> Caio
> 
>