From d57e9aca0137dd088825abd80ea4a616e49d17ed Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Tue, 23 Jun 2020 13:52:21 +0300 Subject: [PATCH] [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 --- llvm/lib/Transforms/Utils/SimplifyIndVar.cpp | 9 ++++++++- llvm/test/Transforms/IndVarSimplify/X86/pr45360.ll | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp b/llvm/lib/Transforms/Utils/SimplifyIndVar.cpp index 05632ca..d3d0c334 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 c4562d81..a100f7b 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 -- 2.7.4