maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Bjorn Pettersson 2021-10-06 12:58:52 +02:00
parent b0c34e0dab
commit 7f93bb4a58
2 changed files with 22 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
View File

@ -103,6 +103,7 @@ static void InsertNewValueIntoMap(ValueToValueMapTy &VM, Value *K, Value *V) {
static void RewriteUsesOfClonedInstructions(BasicBlock *OrigHeader, static void RewriteUsesOfClonedInstructions(BasicBlock *OrigHeader,
BasicBlock *OrigPreheader, BasicBlock *OrigPreheader,
ValueToValueMapTy &ValueMap, ValueToValueMapTy &ValueMap,
ScalarEvolution *SE,
SmallVectorImpl<PHINode*> *InsertedPHIs) { SmallVectorImpl<PHINode*> *InsertedPHIs) {
// Remove PHI node entries that are no longer live. // Remove PHI node entries that are no longer live.
BasicBlock::iterator I, E = OrigHeader->end(); 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 // The value now exits in two versions: the initial value in the preheader
// and the loop "next" value in the original header. // and the loop "next" value in the original header.
SSA.Initialize(OrigHeaderVal->getType(), OrigHeaderVal->getName()); 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(OrigHeader, OrigHeaderVal);
SSA.AddAvailableValue(OrigPreheader, OrigPreHeaderVal); SSA.AddAvailableValue(OrigPreheader, OrigPreHeaderVal);
@ -563,7 +568,7 @@ bool LoopRotate::rotateLoop(Loop *L, bool SimplifiedLatch) {
SmallVector<PHINode*, 2> InsertedPHIs; SmallVector<PHINode*, 2> InsertedPHIs;
// If there were any uses of instructions in the duplicated block outside the // If there were any uses of instructions in the duplicated block outside the
// loop, update them, inserting PHI nodes as required // loop, update them, inserting PHI nodes as required
RewriteUsesOfClonedInstructions(OrigHeader, OrigPreheader, ValueMap, RewriteUsesOfClonedInstructions(OrigHeader, OrigPreheader, ValueMap, SE,
&InsertedPHIs); &InsertedPHIs);
// Attach dbg.value intrinsics to the new phis if that phi uses a value that // Attach dbg.value intrinsics to the new phis if that phi uses a value that

29
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
View File

@ -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),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,CHECK-SCEV-NOK %s ; RUN: opt < %s -passes='print<scalar-evolution>,loop(loop-rotate),print<scalar-evolution>' -disable-output 2>&1 | FileCheck -check-prefixes CHECK-SCEV %s
; FIXME (crashes): opt < %s -passes='loop(canon-freeze),loop(loop-rotate),print<scalar-evolution>' -disable-output ; 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 ; 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 ; analysis cache before loop-rotate. That was enough to see the problem by
; by examining print<scalar-evolution> printouts after loop-rotate. However, ; examining print<scalar-evolution> printouts after loop-rotate. However, the
; I've only seen the crashes when using canon-freeze as a trigger to populate ; crashes where only observed when using canon-freeze as a trigger to populate
; the analysis cache. ; 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. ; Verify that we get the same SCEV expressions after loop-rotate, regardless
; FIXME: As indicated by CHECK-SCEV-OK vs CHECK-SCEV-NOK this isn't currently true (PR51981). ; 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: Classifying expressions for: @test_function
; CHECK-SCEV: %wide = load i32, i32* @offset, align 1 ; 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 = 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: --> %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: %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: --> (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: %iv = phi i16 [ %narrow, %loop.inner.ph ], [ %iv.plus, %loop.inner ] ; 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: --> {(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 }
@offset = external dso_local global i32, align 1 @offset = external dso_local global i32, align 1