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
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
