[PATCH] D39320: [IRCE] Ensure that expanded exit values are available in loop's preheader

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1468
       cast<SCEVConstant>(SE.getConstant(IVTy, -1, true /* isSigned */));
+  const Loop *L = LI.getLoopFor(MainLoopStructure.Header);
 
----------------
To avoid confusion, call this something like MainL or OriginalL.


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1485
 
-    if (!isSafeToExpandAt(ExitPreLoopAtSCEV, InsertPt, SE)) {
+    if (!isSafeToExpandAt(ExitPreLoopAtSCEV, InsertPt, SE) ||
+        !SE.isAvailableAtLoopEntry(ExitPreLoopAtSCEV, L)) {
----------------
Put these in the other order so that we don't run isSafeToExpandAt if the value is not available.  I don't know that it currently does, but isSafeToExpandAt could reasonable have a precondition that the location is available.  :)


================
Comment at: lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp:1512
 
-    if (!isSafeToExpandAt(ExitMainLoopAtSCEV, InsertPt, SE)) {
+    if (!isSafeToExpandAt(ExitMainLoopAtSCEV, InsertPt, SE) ||
+        !SE.isAvailableAtLoopEntry(ExitMainLoopAtSCEV, L)) {
----------------
Same.


================
Comment at: test/Transforms/IRCE/bad_expander.ll:169
+  %gep = getelementptr i64, i64* %p1, i64 %iv.next
+  %loaded = load i64, i64* %gep, align 4
+  %tmp7 = icmp slt i64 %iv.next, 1000
----------------
What is this trying to test?  The potentially faulting load doesn't contribute to any latch or range check, so how would it show up in the split value?


================
Comment at: test/Transforms/IRCE/bad_expander.ll:200
+  %gep = getelementptr i64, i64* %p1, i64 %iv.next
+  %loaded = load i64, i64* %gep, align 4
+  %tmp7 = icmp slt i64 %iv.next, 1000
----------------
Same problem.


https://reviews.llvm.org/D39320