Skip to content

Commit

Permalink
[LoopLoadElim] Filter away candidates that stop being AddRecs after l…
Browse files Browse the repository at this point in the history
…oop 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
  • Loading branch information
xortator committed Sep 10, 2020
1 parent 060c8e0 commit c413a8a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
25 changes: 20 additions & 5 deletions llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
Expand Up @@ -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);

Expand All @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -571,7 +586,7 @@ class LoadEliminationForLoop {
"storeforward");
for (const auto &Cand : Candidates)
propagateStoredValueToLoadUsers(Cand, SEE);
NumLoopLoadEliminted += NumForwarding;
NumLoopLoadEliminted += Candidates.size();

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion 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

Expand Down

0 comments on commit c413a8a

Please sign in to comment.