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

[mkazantsev]

Summary

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.

[asbirlea]

llvm/test/Transforms/LoopLoadElim/pr47457.ll

4

Readd "; REQUIRES: asserts"

[mkazantsev]

llvm/test/Transforms/LoopLoadElim/pr47457.ll

4

Ok, thanks!

llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp

bool processLoop() {
     // To keep things simple for now, remove those where the load is potentially
     // fed by multiple stores.
     removeDependencesFromMultipleStores(StoreToLoadDependences);
     if (StoreToLoadDependences.empty())
       return false;
 
     // Filter the candidates further.
     SmallVector<StoreToLoadForwardingCandidate, 4> Candidates;
-    unsigned NumForwarding = 0;
     for (const StoreToLoadForwardingCandidate &Cand : StoreToLoadDependences) {
       LLVM_DEBUG(dbgs() << "Candidate " << Cand);
 
       // Make sure that the stored values is available everywhere in the loop in
       // the next iteration.
       if (!doesStoreDominatesAllLatches(Cand.Store->getParent(), L, DT))
         continue;
 
       // If the load is conditional we can't hoist its 0-iteration instance to
       // the preheader because that would make it unconditional.  Thus we would
       // access a memory location that the original loop did not access.
       if (isLoadConditional(Cand.Load, L))
         continue;
 
       // Check whether the SCEV difference is the same as the induction step,
       // thus we load the value in the next iteration.
       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;
 
     // Check intervening may-alias stores.  These need runtime checks for alias
     // disambiguation.
     SmallVector<RuntimePointerCheck, 4> Checks = collectMemchecks(Candidates);
 
if (!Checks.empty() || !LAI.getPSE().getUnionPredicate().isAlwaysTrue()) {
 
       // Point of no-return, start the transformation.  First, version the loop
       // if necessary.
 
       LoopVersioning LV(LAI, L, LI, DT, PSE.getSE(), false);
       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.
     // Also for the first iteration, generate the initial value of the load.
     SCEVExpander SEE(*PSE.getSE(), L->getHeader()->getModule()->getDataLayout(),
                      "storeforward");
     for (const auto &Cand : Candidates)
       propagateStoredValueToLoadUsers(Cand, SEE);
-    NumLoopLoadEliminted += NumForwarding;
+    NumLoopLoadEliminted += Candidates.size();
 
     return true;
   }
 
 private:
   Loop *L;
 
   /// Maps the load/store instructions to their index according to

llvm/test/Transforms/LoopLoadElim/pr47457.ll

 ; 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
 
 bb:
   br label %bb1
 
 bb1:                                              ; preds = %bb6, %bb1, %bb
   %tmp = phi i32 [ undef, %bb ], [ 0, %bb1 ], [ %tmp3, %bb6 ]