[PATCH] D110813: [LoopRotate] Forget SCEV values in RewriteUsesOfClonedInstructions

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope created this revision.
bjope added reviewers: fhahn, reames.
Herald added subscribers: javed.absar, hiraditya.
bjope requested review of this revision.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110813

Files:
  llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
  llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll


Index: llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
===================================================================
--- llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
+++ llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
@@ -1,9 +1,13 @@
-; 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),function(bounds-checking)' -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),function(bounds-checking)' -disable-output
 
-; 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_memcpy_1
 ; CHECK-SCEV:   %div210 = load i32, i32* @offset, align 1
@@ -19,11 +23,9 @@
 ; CHECK-SCEV:   %div2102 = phi i32 [ %div2101, %for.body215.lr.ph.lr.ph ], [ %div210, %crit_edge ]
 ; CHECK-SCEV:   -->  %div2102 U: full-set S: full-set         Exits: <<Unknown>>              LoopDispositions: { %for.body215.lr.ph: Variant, %for.body215: Invariant }
 ; CHECK-SCEV:   %conv211 = trunc i32 %div2102 to i16
-; CHECK-SCEV-OK:   -->  (trunc i32 %div2102 to i16) U: full-set S: full-set               Exits: <<Unknown>>              LoopDispositions: { %for.body215.lr.ph: Variant, %for.body215: Invariant }
-; CHECK-SCEV-NOK:   -->  (trunc i32 %div210 to i16) U: full-set S: full-set               Exits: <<Unknown>>              LoopDispositions: { %for.body215.lr.ph: Variant, %for.body215: Invariant }
+; CHECK-SCEV:   -->  (trunc i32 %div2102 to i16) U: full-set S: full-set               Exits: <<Unknown>>              LoopDispositions: { %for.body215.lr.ph: Variant, %for.body215: Invariant }
 ; CHECK-SCEV:   %inc2243 = phi i16 [ %conv211, %for.body215.lr.ph ], [ %inc224, %for.body215 ]
-; CHECK-SCEV-OK:   -->  {(trunc i32 %div2102 to i16),+,1}<%for.body215> U: full-set S: full-set           Exits: (-1 + (700 umax (1 + (trunc i32 %div2102 to i16))))               LoopDispositions: { %for.body215: Computable, %for.body215.lr.ph: Variant }
-; CHECK-SCEV-NOK:   -->  {(trunc i32 %div210 to i16),+,1}<%for.body215> U: full-set S: full-set           Exits: (-1 + (700 umax (1 + (trunc i32 %div210 to i16))))               LoopDispositions: { %for.body215: Computable, %for.body215.lr.ph: Variant }
+; CHECK-SCEV:   -->  {(trunc i32 %div2102 to i16),+,1}<%for.body215> U: full-set S: full-set           Exits: (-1 + (700 umax (1 + (trunc i32 %div2102 to i16))))               LoopDispositions: { %for.body215: Computable, %for.body215.lr.ph: Variant }
 
 
 @offset = external dso_local global i32, align 1
Index: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
+++ llvm/lib/Transforms/Utils/LoopRotationUtils.cpp
@@ -103,6 +103,7 @@
 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 @@
     // 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 @@
     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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D110813.376156.patch
Type: text/x-patch
Size: 5455 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210930/a8e2af87/attachment.bin>