[llvm] d57e9ac - [IndVarSimplify] Don't replace IV user with unsafe loop-invariant (PR45360)

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-06-23T13:53:15+03:00
New Revision: d57e9aca0137dd088825abd80ea4a616e49d17ed

URL: https://github.com/llvm/llvm-project/commit/d57e9aca0137dd088825abd80ea4a616e49d17ed
DIFF: https://github.com/llvm/llvm-project/commit/d57e9aca0137dd088825abd80ea4a616e49d17ed.diff

LOG: [IndVarSimplify] Don't replace IV user with unsafe loop-invariant (PR45360)

Summary:
As [[ https://bugs.llvm.org/show_bug.cgi?id=45360 | PR45360 ]] reports,
with new cost-model we can sometimes end up being able to expand `udiv`/`urem` instructions.
And that exposes at least one instance of when we do that
regardless of whether or not it is safe to do.
In this particular case, it's `SimplifyIndvar::replaceIVUserWithLoopInvariant()`.

It seems to me, we simply need to check with `isSafeToExpandAt()` first.

The test isn't great. I'm not sure how to make it only run `-indvars`.

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=45360 | PR45360 ]].

Reviewers: mkazantsev, reames, helloqirun

Reviewed By: mkazantsev

Subscribers: hiraditya, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D82108

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
    llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
index 05632ca40fb0..d3d0c3341908 100644
--- a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp
@@ -657,7 +657,7 @@ static Instruction *GetLoopInvariantInsertPosition(Loop *L, Instruction *Hint) {
   return Hint;
 }
 
-/// Replace the UseInst with a constant if possible.
+/// Replace the UseInst with a loop invariant expression if it is safe.
 bool SimplifyIndvar::replaceIVUserWithLoopInvariant(Instruction *I) {
   if (!SE->isSCEVable(I->getType()))
     return false;
@@ -673,6 +673,13 @@ bool SimplifyIndvar::replaceIVUserWithLoopInvariant(Instruction *I) {
     return false;
 
   auto *IP = GetLoopInvariantInsertPosition(L, I);
+
+  if (!isSafeToExpandAt(S, IP, *SE)) {
+    LLVM_DEBUG(dbgs() << "INDVARS: Can not replace IV user: " << *I
+                      << " with non-speculable loop invariant: " << *S << '\n');
+    return false;
+  }
+
   auto *Invariant = Rewriter.expandCodeFor(S, I->getType(), IP);
 
   I->replaceAllUsesWith(Invariant);

diff  --git a/llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll b/llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll
index c4562d811d09..a100f7b4e822 100644
--- a/llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll
+++ b/llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll
@@ -26,7 +26,6 @@ define dso_local i32 @main() {
 ; CHECK:       bb19:
 ; CHECK-NEXT:    [[I8_LCSSA9:%.*]] = phi i32 [ [[D_PROMOTED8]], [[BB:%.*]] ], [ [[I8:%.*]], [[BB27:%.*]] ]
 ; CHECK-NEXT:    [[I8]] = and i32 [[I8_LCSSA9]], [[I6]]
-; CHECK-NEXT:    [[TMP0:%.*]] = urem i32 [[I24]], [[I8]]
 ; CHECK-NEXT:    [[I21:%.*]] = icmp eq i32 [[I8]], 0
 ; CHECK-NEXT:    br i1 [[I21]], label [[BB27_THREAD:%.*]], label [[BB27]]
 ; CHECK:       bb27.thread:
@@ -35,8 +34,9 @@ define dso_local i32 @main() {
 ; CHECK-NEXT:    store i32 0, i32* @c, align 4
 ; CHECK-NEXT:    br label [[BB32:%.*]]
 ; CHECK:       bb27:
-; CHECK-NEXT:    store i32 [[TMP0]], i32* @e, align 4
-; CHECK-NEXT:    [[I30:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT:    [[I26:%.*]] = urem i32 [[I24]], [[I8]]
+; CHECK-NEXT:    store i32 [[I26]], i32* @e, align 4
+; CHECK-NEXT:    [[I30:%.*]] = icmp eq i32 [[I26]], 0
 ; CHECK-NEXT:    br i1 [[I30]], label [[BB32_LOOPEXIT:%.*]], label [[BB19]]
 ; CHECK:       bb32.loopexit:
 ; CHECK-NEXT:    store i32 -1, i32* @f, align 4