[PATCH] D137060: [IndVars] Forget the SCEV when the instruction has been sunk.

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

LGTM, thanks!



================
Comment at: llvm/test/Transforms/IndVarSimplify/pr58662.ll:6
+
+define i8 @l(i32 %inc, i1 %tobool.not.i) {
+; CHECK-LABEL: @l(
----------------
It looks like there already SCEV invalidation tests in `llvm/test/Transforms/IndVarSimplify/scev-invalidation.ll`. Could you move the test there?


================
Comment at: llvm/test/Transforms/IndVarSimplify/pr58662.ll:33
+
+for.cond:                                         ; preds = %h.exit, %entry
+  %storemerge = phi i16 [ 0, %entry ], [ %sub6, %h.exit ]
----------------
Naming could be improved to make things easier to read, e.g. `%for.cond` -> `outer.header`, `while.body.i` -> `inner`, `h.exit`-> `outer.latch`.


================
Comment at: llvm/test/Transforms/IndVarSimplify/pr58662.ll:34
+for.cond:                                         ; preds = %h.exit, %entry
+  %storemerge = phi i16 [ 0, %entry ], [ %sub6, %h.exit ]
+  %and = and i32 1, %inc
----------------
`store merge` -> `outer.iv`,  `sub6` -> `outer.iv.next`


================
Comment at: llvm/test/Transforms/IndVarSimplify/pr58662.ll:48
+h.exit:                                           ; preds = %while.body.i
+  %inc.i.lcssa = phi i32 [ 0, %while.body.i ]
+  %0 = trunc i32 %and to i8
----------------
this seems mis-named, maybe just `lcssa`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137060