Skip to content

Conversation

@fhahn
Copy link
Contributor

@fhahn fhahn commented Oct 21, 2025

VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion de-duplication is handled locally in expandSCEVs.

VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion de-duplication is handled locally in expandSCEVs.
@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2025

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Florian Hahn (fhahn)

Changes

VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion de-duplication is handled locally in expandSCEVs.


Full diff: https://github.com/llvm/llvm-project/pull/164490.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Vectorize/VPlan.h (-14)
  • (modified) llvm/lib/Transforms/Vectorize/VPlanUtils.cpp (-3)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h index 167ba553af599..38f49c1a5f84c 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -4166,11 +4166,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. - DenseMap<const SCEV *, VPValue *> SCEVToExpansion; - /// Blocks allocated and owned by the VPlan. They will be deleted once the /// VPlan is destroyed. SmallVector<VPBlockBase *> CreatedBlocks; @@ -4427,15 +4422,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(); diff --git a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp index 32e4b8872b1df..77aee757476e7 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanUtils.cpp @@ -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()); @@ -50,7 +48,6 @@ VPValue *vputils::getOrCreateVPValueForSCEVExpr(VPlan &Plan, const SCEV *Expr) { Plan.getEntry()->appendRecipe(Expanded->getDefiningRecipe()); } } - Plan.addSCEVExpansion(Expr, Expanded); return Expanded; } 
Copy link
Contributor

@artagnon artagnon left a comment

Choose a reason for hiding this comment

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

I actually had this patch initially, but got worried about caching -- on second thought, SCEV internally caches and de-duplicates (maybe add this to the commit message?), so this LGTM, thanks!

@fhahn fhahn enabled auto-merge (squash) October 24, 2025 21:07
@fhahn fhahn merged commit 8c29bce into llvm:main Oct 24, 2025
9 of 10 checks passed
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Oct 24, 2025
VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion de-duplication is handled locally in expandSCEVs. PR: llvm/llvm-project#164490
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 24, 2025

LLVM Buildbot has detected a new failure on builder ppc64le-mlir-rhel-clang running on ppc64le-mlir-rhel-test while building llvm at step 6 "test-build-check-mlir-build-only-check-mlir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/32159

Here is the relevant piece of the build log for the reference
Step 6 (test-build-check-mlir-build-only-check-mlir) failure: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill ... PASS: MLIR-Unit :: IR/./MLIRIRTests/100/130 (3598 of 3609) PASS: MLIR-Unit :: IR/./MLIRIRTests/0/130 (3599 of 3609) PASS: MLIR-Unit :: IR/./MLIRIRTests/101/130 (3600 of 3609) PASS: MLIR-Unit :: IR/./MLIRIRTests/39/130 (3601 of 3609) PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/12/22 (3602 of 3609) PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/13/22 (3603 of 3609) PASS: MLIR-Unit :: IR/./MLIRIRTests/38/130 (3604 of 3609) PASS: MLIR-Unit :: Interfaces/./MLIRInterfacesTests/11/22 (3605 of 3609) PASS: MLIR-Unit :: Pass/./MLIRPassTests/10/13 (3606 of 3609) PASS: MLIR :: mlir-reduce/dce-test.mlir (3607 of 3609) command timed out: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill process killed by signal 9 program finished with exit code -1 elapsedTime=3648.366012 
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment