[Mesa-dev] [PATCH] nir: propagate known constant values into the if-then branch

[Timothy Arceri]

On 23/1/19 7:17 am, Caio Marcelo de Oliveira Filho wrote:
> Hi,
> 
> I like this patch, did it get dropped for a specific reason or just
> forgotten?

Did you look at any of the HURT? The problem I was seeing was this could 
end up stopping copy propagation from working on some UBOs etc.

For example if you have something like:

load UBO at offset offset_a;

if (offset_a == 10) {
    load UBO at offset offset_a;
    ...
}

Without this patch this will get optimised to a single UBO load outside 
the loop.

However with this patch we end up with:

load UBO at offset offset_a;

if (offset_a == 10) {
    load UBO at offset 10;
    ...
}

copy prop no longer recognizes that we are reusing the same UBO and 
reloads it again.

> 
> 
> shader-db results skl:
> 
> total instructions in shared programs: 15049273 -> 15049211 (<.01%)
> instructions in affected programs: 75678 -> 75616 (-0.08%)
> helped: 197
> HURT: 8
> 
> total cycles in shared programs: 369994915 -> 370136090 (0.04%)
> cycles in affected programs: 1750477 -> 1891652 (8.06%)
> helped: 91
> HURT: 91
> 
> total loops in shared programs: 4401 -> 4401 (0.00%)
> loops in affected programs: 0 -> 0
> helped: 0
> HURT: 0
> 
> total spills in shared programs: 10159 -> 10159 (0.00%)
> spills in affected programs: 0 -> 0
> helped: 0
> HURT: 0
> 
> total fills in shared programs: 22118 -> 22118 (0.00%)
> fills in affected programs: 0 -> 0
> helped: 0
> HURT: 0
> 
> LOST:   0
> GAINED: 0
> 
> 
> 
>> +static bool
>> +opt_for_known_values(nir_builder *b, nir_if *nif)
>> +{
>> +   bool progress = false;
> 
> Remove because unused.
> 
> 
>> +      nir_ssa_def *src0 = alu->src[0].src.ssa;
>> +      nir_ssa_def *src1 = alu->src[1].src.ssa;
>> +      if (src0a->parent_instr->type == nir_instr_type_load_const) {
> 
> You fixed this already.
> 
> 
>> +
>> +      break;
>> +   }
>> +
>> +   default:
>> +      return false;
>> +   }
>> +
>> +   return false;
> 
> Optional: unless you plan to add more cases here, consider replacing
> the switch with an if (alu->op == ... || alu->op == ...), so you don't
> have this noise at the end.
> 
> 
> With the 'progress' and the 'src0a' fixes, this patch is
> 
> Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
> 
> 
> 	Caio
>