[llvm] c413a8a - [LoopLoadElim] Filter away candidates that stop being AddRecs after loop versioning. PR47457

[Max Kazantsev via llvm-commits]

Author: Max Kazantsev
Date: 2020-09-10T13:30:31+07:00
New Revision: c413a8a8ecd3c0ef7bcb08525fd73eb1392a738c

URL: https://github.com/llvm/llvm-project/commit/c413a8a8ecd3c0ef7bcb08525fd73eb1392a738c
DIFF: https://github.com/llvm/llvm-project/commit/c413a8a8ecd3c0ef7bcb08525fd73eb1392a738c.diff

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

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.

Differential Revision: https://reviews.llvm.org/D87355
Reviewed By: asbirlea

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp b/llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
index 3b7069564041..e8473d652025 100644
--- a/llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
@@ -486,7 +486,6 @@ class LoadEliminationForLoop {
 
     // Filter the candidates further.
     SmallVector<StoreToLoadForwardingCandidate, 4> Candidates;
-    unsigned NumForwarding = 0;
     for (const StoreToLoadForwardingCandidate &Cand : StoreToLoadDependences) {
       LLVM_DEBUG(dbgs() << "Candidate " << Cand);
 
@@ -506,12 +505,17 @@ class LoadEliminationForLoop {
       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?");
+
+      Candidates.push_back(Cand);
       LLVM_DEBUG(
           dbgs()
-          << NumForwarding
+          << Candidates.size()
           << ". Valid store-to-load forwarding across the loop backedge\n");
-      Candidates.push_back(Cand);
     }
     if (Candidates.empty())
       return false;
@@ -563,6 +567,17 @@ class LoadEliminationForLoop {
       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);
     }
 
     // Next, propagate the value stored by the store to the users of the load.
@@ -571,7 +586,7 @@ class LoadEliminationForLoop {
                      "storeforward");
     for (const auto &Cand : Candidates)
       propagateStoredValueToLoadUsers(Cand, SEE);
-    NumLoopLoadEliminted += NumForwarding;
+    NumLoopLoadEliminted += Candidates.size();
 
     return true;
   }

diff  --git a/llvm/test/Transforms/LoopLoadElim/pr47457.ll b/llvm/test/Transforms/LoopLoadElim/pr47457.ll
index 1b102944cd76..a58be5a8cf5e 100644
--- a/llvm/test/Transforms/LoopLoadElim/pr47457.ll
+++ b/llvm/test/Transforms/LoopLoadElim/pr47457.ll
@@ -1,11 +1,11 @@
 ; 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