[PATCH] D91690: [LoopFlatten] Widen IV, cont'd

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopFlatten.cpp:563
+    if (FI.Widened)
+      if (Value *U = HasSZExtUser(V))
+        V = U;
----------------
dmgreen wrote:
> I'm not sure I understand any more. Should we not be replacing it with trunc(OuterInductionPHI) ?
> 
> Can you add a test where it (the o*I+i value) is not zext or sext?
> I'm not sure I understand any more. Should we not be replacing it with trunc(OuterInductionPHI) ?

After widening we have e.g. this pattern:

  %indvar = phi i64 [ %indvar.next, %for.body3.us ], [ 0, %for.cond1.preheader.us ]
  %3 = trunc i64 %indvar to i32
  %add.us = add i32 %3, %mul.us
  %idxprom.us = zext i32 %add.us to i64

The linear IV user in this example is:

  %add.us = add i32 %3, %mul.us

We don't want to be replacing this `%add.us` value which is a i32 value, because it will indeed be replaced by OuterInductionPhi, which is i64 value after widening. This was the assertion is was talking about. 
After widening, the value that we should be replacing is zext user which is `%idxprom.us` in this case.
After widening, we have this IV -> Trunc->LinearIV ->Ext pattern, which is what we are matching here.

I will add a comment to clarify this.

> Can you add a test where it (the o*I+i value) is not zext or sext?

I think these cases are present in the original test test/Transforms/LoopFlatten/loop-flatten.ll.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91690/new/

https://reviews.llvm.org/D91690