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

@fhahn fhahn deleted the vplan-remove-scevmap branch October 25, 2025 18:21
dvbuka pushed a commit to dvbuka/llvm-project that referenced this pull request Oct 27, 2025
VPlan::SCEVToExpansion isn't needed any longer, as SCEV expansion
de-duplication is handled locally in expandSCEVs.

PR: llvm#164490
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants