[PATCH] D87355: [LoopLoadElim] Filter away candidates that stop being AddRecs after loop versioning. PR47457

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev created this revision.
mkazantsev added reviewers: fhahn, asbirlea, sanjoy.google.
Herald added subscribers: llvm-commits, javed.absar, hiraditya.
Herald added a project: LLVM.
mkazantsev requested review of this revision.

The test in PR47457 demonstrates a situation when candidate load's pointer's SCEV
is no loger a SCEVAddRec after loop versioning. The code there assumes that it is
always a SCEVAddRec and crashes otherwise.

This patch makes sure that we do not consider candidates for which this requirement
is broken after the versioning.


https://reviews.llvm.org/D87355

Files:
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/test/Transforms/LoopLoadElim/pr47457.ll


Index: llvm/test/Transforms/LoopLoadElim/pr47457.ll
===================================================================
--- llvm/test/Transforms/LoopLoadElim/pr47457.ll
+++ llvm/test/Transforms/LoopLoadElim/pr47457.ll
@@ -1,11 +1,10 @@
 ; RUN: opt -loop-load-elim -S %s | FileCheck %s
 ; RUN: opt -passes=loop-load-elim -S %s | FileCheck %s
-; REQUIRES: asserts
-; XFAIL: *
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
 target triple = "x86_64-unknown-linux-gnu"
 
+; Make sure it does not crash with assert.
 define void @test() {
 ; CHECK-LABEL: test
 
Index: llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
+++ llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
@@ -486,7 +486,6 @@
 
     // Filter the candidates further.
     SmallVector<StoreToLoadForwardingCandidate, 4> Candidates;
-    unsigned NumForwarding = 0;
     for (const StoreToLoadForwardingCandidate &Cand : StoreToLoadDependences) {
       LLVM_DEBUG(dbgs() << "Candidate " << Cand);
 
@@ -506,10 +505,15 @@
       if (!Cand.isDependenceDistanceOfOne(PSE, L))
         continue;
 
-      ++NumForwarding;
+      assert(isa<SCEVAddRecExpr>(PSE.getSCEV(Cand.Load->getPointerOperand())) &&
+             "Loading from something other than indvar?");
+      assert(
+          isa<SCEVAddRecExpr>(PSE.getSCEV(Cand.Store->getPointerOperand())) &&
+          "Storing to something other than indvar?");
+
       LLVM_DEBUG(
           dbgs()
-          << NumForwarding
+          << Candidates.size()
           << ". Valid store-to-load forwarding across the loop backedge\n");
       Candidates.push_back(Cand);
     }
@@ -563,6 +567,25 @@
       LV.setAliasChecks(std::move(Checks));
       LV.setSCEVChecks(LAI.getPSE().getUnionPredicate());
       LV.versionLoop();
+
+      // After versioning, some of the candidates' pointers could stop being
+      // SCEVAddRecs. We need to filter them out.
+      auto NoLongerGoodCandidate = [this](
+          const StoreToLoadForwardingCandidate &Cand) {
+        return !isa<SCEVAddRecExpr>(
+                    PSE.getSCEV(Cand.Load->getPointerOperand())) ||
+               !isa<SCEVAddRecExpr>(
+                    PSE.getSCEV(Cand.Store->getPointerOperand()));
+      };
+      llvm::erase_if(Candidates, NoLongerGoodCandidate);
+      for (auto &Cand : Candidates) {
+        assert(
+            isa<SCEVAddRecExpr>(PSE.getSCEV(Cand.Load->getPointerOperand())) &&
+            "AFTER VERSIONING: Loading from something other than indvar?");
+        assert(
+            isa<SCEVAddRecExpr>(PSE.getSCEV(Cand.Store->getPointerOperand())) &&
+            "AFTER VERSIONING: Storing to something other than indvar?");
+      }
     }
 
     // Next, propagate the value stored by the store to the users of the load.
@@ -571,7 +594,7 @@
                      "storeforward");
     for (const auto &Cand : Candidates)
       propagateStoredValueToLoadUsers(Cand, SEE);
-    NumLoopLoadEliminted += NumForwarding;
+    NumLoopLoadEliminted += Candidates.size();
 
     return true;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87355.290674.patch
Type: text/x-patch
Size: 3204 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200909/46535877/attachment.bin>