[llvm] [SLP] Fix spill-cost cache lookup and predecessor scan (PR #192709)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 17 11:36:04 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-vectorizers
Author: Alexey Bataev (alexey-bataev)
<details>
<summary>Changes</summary>
A cached intra-block scan that stopped at a call or budget limit only
proves the sub-range below the stop point is call-free; do not reuse
the cached bit for queries whose First lies above it. Also switch the
cross-block predecessor scan to "exists a call-free backward path"
semantics, skip blocks strictly dominated by Root, and memoize only
the (Root, OpParent) key. Fixes a false-positive spill cost that was
blocking profitable vectorization.
---
Full diff: https://github.com/llvm/llvm-project/pull/192709.diff
3 Files Affected:
- (modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (+73-24)
- (modified) llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-call-between-operands.ll (+4-11)
- (modified) llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-loop-backedge.ll (+1-1)
``````````diff
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index 6625384616c26..7cfdee8da2dfc 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -17538,10 +17538,13 @@ bool BoUpSLP::isTreeNotExtendable() const {
}
InstructionCost BoUpSLP::getSpillCost() {
- // Walk from the bottom of the tree to the top, tracking which values are
- // live. When we see a call instruction that is not part of our tree,
- // query TTI to see if there is a cost to keeping values live over it
- // (for example, if spills and fills are required).
+ // Walk the vectorizable tree from the root towards its leaves, tracking
+ // which vectorized operand values would be live across each tree edge
+ // (i.e. between the last instruction of an operand entry and the last
+ // instruction of its user entry). When the live range crosses a call
+ // instruction that is not part of the vectorized tree, query TTI for the
+ // cost of keeping the value live across it (for example, if spills and
+ // fills are required).
const TreeEntry *Root = VectorizableTree.front().get();
if (Root->isGather())
@@ -17600,10 +17603,19 @@ InstructionCost BoUpSLP::getSpillCost() {
if (auto It = CheckedInstructions.find(Last);
It != CheckedInstructions.end()) {
const Instruction *Checked = It->second.getPointer();
- if (Checked == First || Checked->comesBefore(First))
- return It->second.getInt() != 0;
+ const bool NoCallsInCachedRange = It->second.getInt() != 0;
+ if (Checked == First)
+ return NoCallsInCachedRange;
+ if (Checked->comesBefore(First))
+ // In every cached state (full clean scan, call-found, or
+ // budget-exhausted) the region strictly above `Checked` up to `Last`
+ // was inspected and proved call-free. Since `First` is above
+ // `Checked`, the queried range [First, Last] is contained in that
+ // call-free region, regardless of whether bit is 0 or 1.
+ return true;
Last = Checked;
- } else if (Last == First || Last->comesBefore(First)) {
+ } else if (Last->comesBefore(First)) {
+ // Empty range.
return true;
}
BasicBlock::const_reverse_iterator InstIt =
@@ -17627,11 +17639,16 @@ InstructionCost BoUpSLP::getSpillCost() {
++PrevInstIt;
++Budget;
}
+ // If we reached the scan's lower bound (`PrevInstIt == InstIt`) then the
+ // whole [First, Last] range was inspected and found call-free, even if
+ // Budget just overflowed at the very last step; do not mislabel such a
+ // completed scan as "has call".
+ const bool Completed = PrevInstIt == InstIt;
+ const bool NoCallsInRange = Completed || Budget <= BudgetLimit;
for (const Instruction *LastInst : LastInstsInRange)
CheckedInstructions.try_emplace(
- LastInst, PrevInstIt == InstIt ? First : &*PrevInstIt,
- Budget <= BudgetLimit ? 1 : 0);
- return Budget <= BudgetLimit;
+ LastInst, Completed ? First : &*PrevInstIt, NoCallsInRange ? 1 : 0);
+ return NoCallsInRange;
};
auto AddCosts = [&](const TreeEntry *Op) {
if (ScalarOrPseudoEntries.contains(Op))
@@ -17670,38 +17687,70 @@ InstructionCost BoUpSLP::getSpillCost() {
SmallPtrSet<const BasicBlock *, 16> Visited;
SmallDenseSet<std::pair<const BasicBlock *, const BasicBlock *>>
ParentsPairsToAdd;
+ // With "at least one call-free path" semantics we can only reliably
+ // memoize the exact (Root, OpParent) query. Pairs for intermediate
+ // blocks that were visited during the BFS are not necessarily
+ // call-free-reachable to OpParent themselves - we may have reached
+ // OpParent through a *sibling* path that bypassed them.
bool Res = false;
- llvm::scope_exit Cleanup([&]() {
- for (const auto &KeyPair : ParentsPairsToAdd) {
- assert(!ParentOpParentToPreds.contains(KeyPair) &&
- "Should not have been added before.");
- ParentOpParentToPreds.try_emplace(KeyPair, Res);
- }
- });
+ llvm::scope_exit Cleanup(
+ [&]() { ParentOpParentToPreds.try_emplace(Key, Res); });
+ // We return `true` (no spill cost) if at least one backward path from
+ // some predecessor of Root back to OpParent is call-free. Only when
+ // *every* such path goes through a non-vec call do we charge the spill
+ // cost: only then is it actually necessary to keep the vectorized value
+ // live across a call and therefore spill/reload it.
+ //
+ // A BB is only explored further (its predecessors added to the worklist)
+ // when it is itself call-free and not strictly dominated by Root (blocks
+ // dominated by Root are only reachable via loop back-edges - they sit
+ // *after* Root in forward execution and must not be counted).
+ //
+ // If we ever pop OpParent from the worklist, we have reached it through
+ // a chain of call-free, non-dominated blocks: a call-free path exists
+ // and we return true. If the worklist is exhausted without reaching
+ // OpParent, every admissible path is blocked by a call and we return
+ // false so the caller charges the spill cost.
while (!Worklist.empty()) {
BasicBlock *BB = Worklist.pop_back_val();
- if (BB == OpParent || !Visited.insert(BB).second)
+ if (BB == OpParent) {
+ Res = true;
+ return Res;
+ }
+ if (!Visited.insert(BB).second)
+ continue;
+ // Blocks strictly dominated by Root are reached only *after* Root in
+ // forward execution (via loop back-edges); skip them and their
+ // dominated predecessors.
+ if (DT->properlyDominates(Root, BB))
continue;
auto Pair = std::make_pair(BB, OpParent);
if (auto It = ParentOpParentToPreds.find(Pair);
It != ParentOpParentToPreds.end()) {
- Res = It->second;
- return Res;
+ if (It->second) {
+ // BB is known to reach OpParent via a call-free path.
+ Res = true;
+ return Res;
+ }
+ // BB is known to be blocked from OpParent by calls; keep checking
+ // other paths.
+ continue;
}
- ParentsPairsToAdd.insert(Pair);
unsigned BlockSize = BB->size();
if (BlockSize > static_cast<unsigned>(ScheduleRegionSizeBudget))
- return Res;
+ continue;
Budget += BlockSize;
if (Budget > BudgetLimit)
return Res;
if (!isa<CatchSwitchInst>(BB->getTerminator()) &&
!CheckForNonVecCallsInSameBlock(&*BB->getFirstNonPHIOrDbgOrAlloca(),
BB->getTerminator()))
- return Res;
+ continue;
Worklist.append(pred_begin(BB), pred_end(BB));
}
- Res = true;
+ // Worklist drained without ever reaching OpParent: every path between
+ // Root and OpParent is blocked by a non-vec call.
+ Res = false;
return Res;
};
SmallVector<const TreeEntry *> LiveEntries(1, Root);
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-call-between-operands.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-call-between-operands.ll
index 262977584a11d..0de6be442daa4 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-call-between-operands.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-call-between-operands.ll
@@ -7,19 +7,12 @@ define void @test(ptr %p, ptr %q, ptr %r) {
; CHECK-LABEL: define void @test(
; CHECK-SAME: ptr [[P:%.*]], ptr [[Q:%.*]], ptr [[R:%.*]]) {
; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[A0:%.*]] = load double, ptr [[P]], align 8
-; CHECK-NEXT: [[PP1:%.*]] = getelementptr inbounds double, ptr [[P]], i64 1
-; CHECK-NEXT: [[A1:%.*]] = load double, ptr [[PP1]], align 8
+; CHECK-NEXT: [[TMP0:%.*]] = load <2 x double>, ptr [[P]], align 8
; CHECK-NEXT: call void @external()
-; CHECK-NEXT: [[B0:%.*]] = load double, ptr [[Q]], align 8
-; CHECK-NEXT: [[QQ1:%.*]] = getelementptr inbounds double, ptr [[Q]], i64 1
-; CHECK-NEXT: [[B1:%.*]] = load double, ptr [[QQ1]], align 8
-; CHECK-NEXT: [[S0:%.*]] = fadd double [[A0]], [[B0]]
-; CHECK-NEXT: [[S1:%.*]] = fadd double [[A1]], [[B1]]
+; CHECK-NEXT: [[TMP1:%.*]] = load <2 x double>, ptr [[Q]], align 8
+; CHECK-NEXT: [[TMP2:%.*]] = fadd <2 x double> [[TMP0]], [[TMP1]]
; CHECK-NEXT: call void @external()
-; CHECK-NEXT: store double [[S0]], ptr [[R]], align 8
-; CHECK-NEXT: [[RR1:%.*]] = getelementptr inbounds double, ptr [[R]], i64 1
-; CHECK-NEXT: store double [[S1]], ptr [[RR1]], align 8
+; CHECK-NEXT: store <2 x double> [[TMP2]], ptr [[R]], align 8
; CHECK-NEXT: ret void
;
entry:
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-loop-backedge.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-loop-backedge.ll
index d820ac30631a8..902d2a7f31327 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-loop-backedge.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/spillcost-loop-backedge.ll
@@ -10,7 +10,7 @@ declare void @external_call()
; YAML-NEXT: Function: test_spillcost_backedge
; YAML-NEXT: Args:
; YAML-NEXT: - String: 'Stores SLP vectorized with cost '
-; YAML-NEXT: - Cost: '-99'
+; YAML-NEXT: - Cost: '-101'
; YAML-NEXT: - String: ' and with tree size '
; YAML-NEXT: - TreeSize: '6'
; YAML-NEXT: ...
``````````
</details>
https://github.com/llvm/llvm-project/pull/192709
More information about the llvm-commits
mailing list