[llvm] 7f93bb4 - [LoopRotate] Forget SCEV values in RewriteUsesOfClonedInstructions
Bjorn Pettersson via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 7 10:56:19 PDT 2021
Author: Bjorn Pettersson
Date: 2021-10-07T19:36:30+02:00
New Revision: 7f93bb4a5827ffce67a469da3ac0e23194538441
URL: https://github.com/llvm/llvm-project/commit/7f93bb4a5827ffce67a469da3ac0e23194538441
DIFF: https://github.com/llvm/llvm-project/commit/7f93bb4a5827ffce67a469da3ac0e23194538441.diff
LOG: [LoopRotate] Forget SCEV values in RewriteUsesOfClonedInstructions
This patch fixes problems reported in PR51981.
When rotating a loop it isn't enough to just forget SCEV for that
loop nest. When rotating we might clone some instructions from the
old header into the preheader, and insert new PHI nodes to merge
values together. There could be users of the original value that are
updated to use the PHI result. And those users were not necessarily
depending on a PHI node earlier, so they weren't cleaned up when just
forgetting all SCEV:s for the loop nest. So we need to explicitly
forget those values to avoid invalid cached SCEV expressions.
Reviewed By: fhahn, mkazantsev
Differential Revision: https://reviews.llvm.org/D110813
Added:
Modified:
llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
index c5b9d2cb9cdb7..692e60a9701e0 100644
--- a/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -103,6 +103,7 @@ static void InsertNewValueIntoMap(ValueToValueMapTy &VM, Value *K, Value *V) {
static void RewriteUsesOfClonedInstructions(BasicBlock *OrigHeader,
BasicBlock *OrigPreheader,
ValueToValueMapTy &ValueMap,
+ ScalarEvolution *SE,
SmallVectorImpl<PHINode*> *InsertedPHIs) {
// Remove PHI node entries that are no longer live.
BasicBlock::iterator I, E = OrigHeader->end();
@@ -125,6 +126,10 @@ static void RewriteUsesOfClonedInstructions(BasicBlock *OrigHeader,
// The value now exits in two versions: the initial value in the preheader
// and the loop "next" value in the original header.
SSA.Initialize(OrigHeaderVal->getType(), OrigHeaderVal->getName());
+ // Force re-computation of OrigHeaderVal, as some users now need to use the
+ // new PHI node.
+ if (SE)
+ SE->forgetValue(OrigHeaderVal);
SSA.AddAvailableValue(OrigHeader, OrigHeaderVal);
SSA.AddAvailableValue(OrigPreheader, OrigPreHeaderVal);
@@ -563,7 +568,7 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
SmallVector<PHINode*, 2> InsertedPHIs;
// If there were any uses of instructions in the duplicated block outside the
// loop, update them, inserting PHI nodes as required
- RewriteUsesOfClonedInstructions(OrigHeader, OrigPreheader, ValueMap,
+ RewriteUsesOfClonedInstructions(OrigHeader, OrigPreheader, ValueMap, SE,
&InsertedPHIs);
// Attach dbg.value intrinsics to the new phis if that phi uses a value that
diff --git a/llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll b/llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
index fd15609301f7a..2df3023b37ad0 100644
--- a/llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
+++ b/llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
@@ -1,15 +1,20 @@
-; RUN: opt < %s -passes='print<scalar-evolution>,loop(loop-rotate),invalidate<scalar-evolution>,print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefixes CHECK-SCEV,CHECK-SCEV-OK %s
-; RUN: opt < %s -passes='print<scalar-evolution>,loop(loop-rotate),print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefixes CHECK-SCEV,CHECK-SCEV-NOK %s
-; FIXME (crashes): opt < %s -passes='loop(canon-freeze),loop(loop-rotate),print<scalar-evolution>' -disable-output
+; RUN: opt < %s -passes='print<scalar-evolution>,loop(loop-rotate),invalidate<scalar-evolution>,print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefixes CHECK-SCEV %s
+; RUN: opt < %s -passes='print<scalar-evolution>,loop(loop-rotate),print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefixes CHECK-SCEV %s
+; RUN: opt < %s -passes='loop(canon-freeze),loop(loop-rotate),print<scalar-evolution>' -disable-output
; In the first two RUN lines print<scalar-evolution> is used to populate the
-; analysis cache before loop-rotate. That seem to be enough to see the problem
-; by examining print<scalar-evolution> printouts after loop-rotate. However,
-; I've only seen the crashes when using canon-freeze as a trigger to populate
-; the analysis cache.
+; analysis cache before loop-rotate. That was enough to see the problem by
+; examining print<scalar-evolution> printouts after loop-rotate. However, the
+; crashes where only observed when using canon-freeze as a trigger to populate
+; the analysis cache, so that is why canon-freeze is used in the third RUN
+; line.
-; Verify that we get the same SCEV expressions after loop-rotate, regardless if we invalidate scalar-evolution before the final printing or not.
-; FIXME: As indicated by CHECK-SCEV-OK vs CHECK-SCEV-NOK this isn't currently true (PR51981).
+; Verify that we get the same SCEV expressions after loop-rotate, regardless
+; if we invalidate scalar-evolution before the final printing or not.
+;
+; This used to fail as described by PR51981 (some expressions still referred
+; to (trunc i32 %div210 to i16) but after the rotation it should be (trunc i32
+; %div2102 to i16).
;
; CHECK-SCEV: Classifying expressions for: @test_function
; CHECK-SCEV: %wide = load i32, i32* @offset, align 1
@@ -25,11 +30,9 @@
; CHECK-SCEV: %wide2 = phi i32 [ %wide1, %loop.inner.ph.lr.ph ], [ %wide, %loop.outer.latch ]
; CHECK-SCEV: --> %wide2 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.inner.ph: Variant, %loop.inner: Invariant }
; CHECK-SCEV: %narrow = trunc i32 %wide2 to i16
-; CHECK-SCEV-OK: --> (trunc i32 %wide2 to i16) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.inner.ph: Variant, %loop.inner: Invariant }
-; CHECK-SCEV-NOK: --> (trunc i32 %wide to i16) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.inner.ph: Variant, %loop.inner: Invariant }
+; CHECK-SCEV: --> (trunc i32 %wide2 to i16) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %loop.inner.ph: Variant, %loop.inner: Invariant }
; CHECK-SCEV: %iv = phi i16 [ %narrow, %loop.inner.ph ], [ %iv.plus, %loop.inner ]
-; CHECK-SCEV-OK: --> {(trunc i32 %wide2 to i16),+,1}<%loop.inner> U: full-set S: full-set Exits: (-1 + (700 umax (1 + (trunc i32 %wide2 to i16)))) LoopDispositions: { %loop.inner: Computable, %loop.inner.ph: Variant }
-; CHECK-SCEV-NOK: --> {(trunc i32 %wide to i16),+,1}<%loop.inner> U: full-set S: full-set Exits: (-1 + (700 umax (1 + (trunc i32 %wide to i16)))) LoopDispositions: { %loop.inner: Computable, %loop.inner.ph: Variant }
+; CHECK-SCEV: --> {(trunc i32 %wide2 to i16),+,1}<%loop.inner> U: full-set S: full-set Exits: (-1 + (700 umax (1 + (trunc i32 %wide2 to i16)))) LoopDispositions: { %loop.inner: Computable, %loop.inner.ph: Variant }
@offset = external dso_local global i32, align 1
More information about the llvm-commits
mailing list