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

[Caio Marcelo de Oliveira Filho]

Hi,

> > The different problem I've found was that uses in the phi instruction
> > after the then/else blocks was being replaced, causing churn in the
> > optimizations further on.  As a hack, I've ignored phi instructions in
> > the use loop above.  HURTs are gone and HELPs continued.
> > 
> > The real issue seems to be use_block not being what we expect when we
> > have a use_src from a phi instruction.  For example, in the snippet
> > below
> > 
> >      vec1 1 ssa_2543 = ieq ssa_7429, ssa_6
> >      /* succs: block_9 block_10 */
> >      if ssa_2543 {
> >              block block_9:
> >              /* preds: block_8 */
> >              vec2 32 ssa_2550 = vec2 ssa_7744, ssa_7745
> >              vec4 32 ssa_2553 = txl ssa_468 (texture_deref), ssa_468 (sampler_deref), ssa_2550 (coord), ssa_6 (lod),
> >              vec1 32 ssa_7434 = imov ssa_2553.w
> >              /* succs: block_11 */
> >      } else {
> >              block block_10:
> >              /* preds: block_8 */
> >              /* succs: block_11 */
> >      }
> >      block block_11:
> >      /* preds: block_9 block_10 */
> >      vec1 32 ssa_7433 = phi block_9: ssa_7420, block_10: ssa_7420
> >      vec1 32 ssa_7436 = phi block_9: ssa_7434, block_10: ssa_7423
> >      vec1 32 ssa_7439 = phi block_9: ssa_7426, block_10: ssa_7426
> >      vec1 32 ssa_7442 = phi block_9: ssa_7429, block_10: ssa_7429
> > 
> > the use_src that is part of the instruction "... ssa_7442 = phi ...",
> > was being considered use_block == 9 (the 'then'-block), so replace was
> > happening.
> 
> I'm not sure I see the problem, replacing the phi src is valid. The question
> is why is it causing HURT?

What happens is that the phi becomes

    vec1 32 ssa_7442 = phi block_9: ssa_6, block_10: ssa_7429

instead of remaining

    vec1 32 ssa_7442 = phi block_9: ssa_7429, block_10: ssa_7429

and the next pass that cause effect (in this case) is
nir_opt_remove_phis, which will not be able to remove this phi.
Further passes will still keep this phi around, and it sticks until
SSA work is done.  This is the SSA form (replacing the phi, 23 NIR
instructions)

    block block_11:
    /* preds: block_9 block_10 */
    vec1 32 ssa_127 = phi block_9: ssa_126, block_10: ssa_101
    vec1 32 ssa_128 = phi block_9: ssa_0, block_10: ssa_104    <<<<<
    vec1 32 ssa_129 = fadd -ssa_96, ssa_122
    vec1 32 ssa_130 = b32csel ssa_106, ssa_122, ssa_129
    vec1 32 ssa_131 = fadd -ssa_96, ssa_127
    vec1 32 ssa_132 = ine32 ssa_128, ssa_0
    vec1 32 ssa_133 = b32csel ssa_132, ssa_127, ssa_131
    vec1 32 ssa_134 = fge32 abs(ssa_133), ssa_95
    vec1 32 ssa_135 = b2i32 ssa_134
    vec1 32 ssa_136 = imov -ssa_135
    vec1 32 ssa_137 = ffma -ssa_78, ssa_6, ssa_107
    vec1 32 ssa_138 = fge32 abs(ssa_130), ssa_95
    vec1 32 ssa_139 = b32csel ssa_138, ssa_107, ssa_137
    vec1 32 ssa_140 = ffma -ssa_79, ssa_6, ssa_109
    vec1 32 ssa_141 = b32csel ssa_138, ssa_109, ssa_140
    vec1 32 ssa_142 = iand ssa_134, ssa_138
    vec1 32 ssa_143 = b2i32 ssa_142
    vec1 32 ssa_144 = ffma ssa_78, ssa_6, ssa_113
    vec1 32 ssa_145 = b32csel ssa_134, ssa_113, ssa_144
    vec1 32 ssa_146 = ffma ssa_79, ssa_6, ssa_115
    vec1 32 ssa_147 = b32csel ssa_134, ssa_115, ssa_146
    vec1 32 ssa_148 = inot -ssa_143
    vec1 32 ssa_149 = ine32 ssa_148, ssa_0
    /* succs: block_12 block_37 */

compare to skipping the phi (19 NIR intructions)

    block block_11:
    /* preds: block_9 block_10 */
    vec1 32 ssa_125 = phi block_9: ssa_124, block_10: ssa_101
    vec1 32 ssa_126 = fadd -ssa_96, ssa_120
    vec1 32 ssa_127 = b32csel ssa_103, ssa_120, ssa_126
    vec1 32 ssa_128 = fadd -ssa_96, ssa_125
    vec1 32 ssa_129 = b32csel ssa_107, ssa_125, ssa_128
    vec1 32 ssa_130 = ffma -ssa_78, ssa_6, ssa_104
    vec1 32 ssa_131 = fge32 abs(ssa_127), ssa_95
    vec1 32 ssa_132 = b32csel ssa_131, ssa_104, ssa_130
    vec1 32 ssa_133 = ffma -ssa_79, ssa_6, ssa_106
    vec1 32 ssa_134 = b32csel ssa_131, ssa_106, ssa_133
    vec1 32 ssa_135 = fge32 abs(ssa_129), ssa_95
    vec1 32 ssa_136 = iand ssa_135, ssa_131
    vec1 32 ssa_137 = b2i32 ssa_136
    vec1 32 ssa_138 = ffma ssa_78, ssa_6, ssa_111
    vec1 32 ssa_139 = b32csel ssa_135, ssa_111, ssa_138
    vec1 32 ssa_140 = ffma ssa_79, ssa_6, ssa_113
    vec1 32 ssa_141 = b32csel ssa_135, ssa_113, ssa_140
    vec1 32 ssa_142 = inot -ssa_137
    vec1 32 ssa_143 = ine32 ssa_142, ssa_0
    /* succs: block_12 block_37 */

There could be other interactions preventing it to get rid of that I'm
missing.  But I think a good approach here could be: if there's
another source in the phi that is the same as the one being replaced,
just skip it since it would prevent the phi being directly removed.


	Caio