Skip to content

[VPlan] Remove SCEVToExpansion mapping (NFC). #164490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 24, 2025
Merged

[VPlan] Remove SCEVToExpansion mapping (NFC). #164490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2f4983
[VPlan] Remove SCEVToExpansion mapping (NFC).
fhahn Oct 21, 2025
08f7860
Merge branch 'main' into vplan-remove-scevmap
fhahn Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions llvm/lib/Transforms/Vectorize/VPlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4172,11 +4172,6 @@ class VPlan {
/// definitions are VPValues that hold a pointer to their underlying IR.
SmallVector<VPValue *, 16> VPLiveIns;

/// Mapping from SCEVs to the VPValues representing their expansions.
/// NOTE: This mapping is temporary and will be removed once all users have
/// been modeled in VPlan directly.
Comment on lines -4176 to -4177
Copy link
Collaborator

@ayalz ayalz Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this NOTE been taken care of, i.e., all users are modeled in VPlan directly?

DenseMap<const SCEV *, VPValue *> SCEVToExpansion;

/// Blocks allocated and owned by the VPlan. They will be deleted once the
/// VPlan is destroyed.
SmallVector<VPBlockBase *> CreatedBlocks;
Expand Down Expand Up @@ -4424,15 +4419,6 @@ class VPlan {
LLVM_DUMP_METHOD void dump() const;
#endif

VPValue *getSCEVExpansion(const SCEV *S) const {
return SCEVToExpansion.lookup(S);
}

void addSCEVExpansion(const SCEV *S, VPValue *V) {
assert(!SCEVToExpansion.contains(S) && "SCEV already expanded");
SCEVToExpansion[S] = V;
}

/// Clone the current VPlan, update all VPValues of the new VPlan and cloned
/// recipes to refer to the clones, and return it.
VPlan *duplicate();
Expand Down
3 changes: 0 additions & 3 deletions llvm/lib/Transforms/Vectorize/VPlanUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ bool vputils::onlyScalarValuesUsed(const VPValue *Def) {
}

VPValue *vputils::getOrCreateVPValueForSCEVExpr(VPlan &Plan, const SCEV *Expr) {
if (auto *Expanded = Plan.getSCEVExpansion(Expr))
return Expanded;
VPValue *Expanded = nullptr;
if (auto *E = dyn_cast<SCEVConstant>(Expr))
Expanded = Plan.getOrAddLiveIn(E->getValue());
Comment on lines 35 to 37
Copy link
Collaborator

@ayalz ayalz Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can now early-exit here, and below:

Suggested change
VPValue *Expanded = nullptr;
if (auto *E = dyn_cast<SCEVConstant>(Expr))
Expanded = Plan.getOrAddLiveIn(E->getValue());
if (auto *E = dyn_cast<SCEVConstant>(Expr))
return Plan.getOrAddLiveIn(E->getValue());

Expand All @@ -50,7 +48,6 @@ VPValue *vputils::getOrCreateVPValueForSCEVExpr(VPlan &Plan, const SCEV *Expr) {
Plan.getEntry()->appendRecipe(Expanded->getDefiningRecipe());
Copy link
Collaborator

@ayalz ayalz Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the previous mapping also took care of caching VPExpandSCEVRecipe's, i.e., if getOrCreateVPValueForSCEVExpr() is called twice for the same EXPR. But such cases should be easy to clean up?

Should ExpandedSCEVs be checked if it already has a Value for Expr, before expanding code for it?

}
}
Plan.addSCEVExpansion(Expr, Expanded);
return Expanded;
}

Expand Down
Loading