[LVI][SCCP] Avoid copying ValueLatticeElement #163901

dtcxzyw · 2025-10-17T02:40:22Z

Copying ValueLatticeElement becomes expensive after #111544. This patch eliminates some redundant copies to improve performance. The code change has been carefully reviewed to ensure that there is no dangling reference.

Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=f4359301c033694d36865c7560714164d2050240&to=4ea449bd53feef43403c35d8b815ddca752dbc17&stat=instructions%3Au

llvmbot · 2025-10-17T02:41:03Z

@llvm/pr-subscribers-function-specialization
@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Copying ValueLatticeElement becomes expensive after #111544. This patch eliminates some redundant copies to improve performance. The code change has been carefully reviewed to ensure that there is no dangling reference.

Compile-time improvement: https://llvm-compile-time-tracker.com/compare.php?from=f4359301c033694d36865c7560714164d2050240&to=ad2cdec8b183fccc500169d8d468cca91edf0e0a&stat=instructions%3Au

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

2 Files Affected:

(modified) llvm/lib/Analysis/LazyValueInfo.cpp (+4-6)
(modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+25-22)

diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 0e5bc481383a0..df75999eb6080 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -947,9 +947,8 @@ LazyValueInfoImpl::solveBlockValueSelect(SelectInst *SI, BasicBlock *BB) {
                                                   /*UseBlockValue*/ false));
   }
 
-  ValueLatticeElement Result = TrueVal;
-  Result.mergeIn(FalseVal);
-  return Result;
+  TrueVal.mergeIn(FalseVal);
+  return TrueVal;
 }
 
 std::optional<ConstantRange>
@@ -1778,9 +1777,8 @@ ValueLatticeElement LazyValueInfoImpl::getValueInBlock(Value *V, BasicBlock *BB,
     assert(OptResult && "Value not available after solving");
   }
 
-  ValueLatticeElement Result = *OptResult;
-  LLVM_DEBUG(dbgs() << "  Result = " << Result << "\n");
-  return Result;
+  LLVM_DEBUG(dbgs() << "  Result = " << *OptResult << "\n");
+  return *OptResult;
 }
 
 ValueLatticeElement LazyValueInfoImpl::getValueAt(Value *V, Instruction *CxtI) {
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 9693ae6b8ceb5..a82c02cbc8468 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -634,11 +634,11 @@ class SCCPInstVisitor : public InstVisitor<SCCPInstVisitor> {
   /// Merge \p MergeWithV into \p IV and push \p V to the worklist, if \p IV
   /// changes.
   bool mergeInValue(ValueLatticeElement &IV, Value *V,
-                    ValueLatticeElement MergeWithV,
+                    const ValueLatticeElement &MergeWithV,
                     ValueLatticeElement::MergeOptions Opts = {
                         /*MayIncludeUndef=*/false, /*CheckWiden=*/false});
 
-  bool mergeInValue(Value *V, ValueLatticeElement MergeWithV,
+  bool mergeInValue(Value *V, const ValueLatticeElement &MergeWithV,
                     ValueLatticeElement::MergeOptions Opts = {
                         /*MayIncludeUndef=*/false, /*CheckWiden=*/false}) {
     assert(!V->getType()->isStructTy() &&
@@ -1128,8 +1128,7 @@ bool SCCPInstVisitor::isStructLatticeConstant(Function *F, StructType *STy) {
   for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
     const auto &It = TrackedMultipleRetVals.find(std::make_pair(F, i));
     assert(It != TrackedMultipleRetVals.end());
-    ValueLatticeElement LV = It->second;
-    if (!SCCPSolver::isConstant(LV))
+    if (!SCCPSolver::isConstant(It->second))
       return false;
   }
   return true;
@@ -1160,7 +1159,7 @@ Constant *SCCPInstVisitor::getConstantOrNull(Value *V) const {
     std::vector<Constant *> ConstVals;
     auto *ST = cast<StructType>(V->getType());
     for (unsigned I = 0, E = ST->getNumElements(); I != E; ++I) {
-      ValueLatticeElement LV = LVs[I];
+      const ValueLatticeElement &LV = LVs[I];
       ConstVals.push_back(SCCPSolver::isConstant(LV)
                               ? getConstant(LV, ST->getElementType(I))
                               : UndefValue::get(ST->getElementType(I)));
@@ -1225,7 +1224,7 @@ void SCCPInstVisitor::visitInstruction(Instruction &I) {
 }
 
 bool SCCPInstVisitor::mergeInValue(ValueLatticeElement &IV, Value *V,
-                                   ValueLatticeElement MergeWithV,
+                                   const ValueLatticeElement &MergeWithV,
                                    ValueLatticeElement::MergeOptions Opts) {
   if (IV.mergeIn(MergeWithV, Opts)) {
     pushUsersToWorkList(V);
@@ -1264,7 +1263,7 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
       return;
     }
 
-    ValueLatticeElement BCValue = getValueState(BI->getCondition());
+    const ValueLatticeElement &BCValue = getValueState(BI->getCondition());
     ConstantInt *CI = getConstantInt(BCValue, BI->getCondition()->getType());
     if (!CI) {
       // Overdefined condition variables, and branches on unfoldable constant
@@ -1326,7 +1325,7 @@ void SCCPInstVisitor::getFeasibleSuccessors(Instruction &TI,
   // the target as executable.
   if (auto *IBR = dyn_cast<IndirectBrInst>(&TI)) {
     // Casts are folded by visitCastInst.
-    ValueLatticeElement IBRValue = getValueState(IBR->getAddress());
+    const ValueLatticeElement &IBRValue = getValueState(IBR->getAddress());
     BlockAddress *Addr = dyn_cast_or_null<BlockAddress>(
         getConstant(IBRValue, IBR->getAddress()->getType()));
     if (!Addr) { // Overdefined or unknown condition?
@@ -1408,7 +1407,7 @@ void SCCPInstVisitor::visitPHINode(PHINode &PN) {
     if (!isEdgeFeasible(PN.getIncomingBlock(i), PN.getParent()))
       continue;
 
-    ValueLatticeElement IV = getValueState(PN.getIncomingValue(i));
+    const ValueLatticeElement &IV = getValueState(PN.getIncomingValue(i));
     PhiState.mergeIn(IV);
     NumActiveIncoming++;
     if (PhiState.isOverdefined())
@@ -1481,7 +1480,7 @@ void SCCPInstVisitor::visitCastInst(CastInst &I) {
     }
   }
 
-  ValueLatticeElement OpSt = getValueState(I.getOperand(0));
+  const ValueLatticeElement &OpSt = getValueState(I.getOperand(0));
   if (OpSt.isUnknownOrUndef())
     return;
 
@@ -1496,9 +1495,9 @@ void SCCPInstVisitor::visitCastInst(CastInst &I) {
   if (I.getDestTy()->isIntOrIntVectorTy() &&
       I.getSrcTy()->isIntOrIntVectorTy() &&
       I.getOpcode() != Instruction::BitCast) {
-    auto &LV = getValueState(&I);
     ConstantRange OpRange =
         OpSt.asConstantRange(I.getSrcTy(), /*UndefAllowed=*/false);
+    auto &LV = getValueState(&I);
 
     Type *DestTy = I.getDestTy();
     ConstantRange Res = ConstantRange::getEmpty(DestTy->getScalarSizeInBits());
@@ -1516,15 +1515,20 @@ void SCCPInstVisitor::handleExtractOfWithOverflow(ExtractValueInst &EVI,
                                                   const WithOverflowInst *WO,
                                                   unsigned Idx) {
   Value *LHS = WO->getLHS(), *RHS = WO->getRHS();
-  ValueLatticeElement L = getValueState(LHS);
-  ValueLatticeElement R = getValueState(RHS);
+  Type *Ty = LHS->getType();
+
   addAdditionalUser(LHS, &EVI);
   addAdditionalUser(RHS, &EVI);
-  if (L.isUnknownOrUndef() || R.isUnknownOrUndef())
-    return; // Wait to resolve.
 
-  Type *Ty = LHS->getType();
+  const ValueLatticeElement &L = getValueState(LHS);
+  if (L.isUnknownOrUndef())
+    return; // Wait to resolve.
   ConstantRange LR = L.asConstantRange(Ty, /*UndefAllowed=*/false);
+
+  const ValueLatticeElement &R = getValueState(RHS);
+  if (R.isUnknownOrUndef())
+    return; // Wait to resolve.
+
   ConstantRange RR = R.asConstantRange(Ty, /*UndefAllowed=*/false);
   if (Idx == 0) {
     ConstantRange Res = LR.binaryOp(WO->getBinaryOp(), RR);
@@ -1616,7 +1620,7 @@ void SCCPInstVisitor::visitSelectInst(SelectInst &I) {
   if (ValueState[&I].isOverdefined())
     return (void)markOverdefined(&I);
 
-  ValueLatticeElement CondValue = getValueState(I.getCondition());
+  const ValueLatticeElement &CondValue = getValueState(I.getCondition());
   if (CondValue.isUnknownOrUndef())
     return;
 
@@ -1802,7 +1806,7 @@ void SCCPInstVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
   Operands.reserve(I.getNumOperands());
 
   for (unsigned i = 0, e = I.getNumOperands(); i != e; ++i) {
-    ValueLatticeElement State = getValueState(I.getOperand(i));
+    const ValueLatticeElement &State = getValueState(I.getOperand(i));
     if (State.isUnknownOrUndef())
       return; // Operands are not resolved yet.
 
@@ -1881,14 +1885,13 @@ void SCCPInstVisitor::visitLoadInst(LoadInst &I) {
   if (ValueState[&I].isOverdefined())
     return (void)markOverdefined(&I);
 
-  ValueLatticeElement PtrVal = getValueState(I.getOperand(0));
+  const ValueLatticeElement &PtrVal = getValueState(I.getOperand(0));
   if (PtrVal.isUnknownOrUndef())
     return; // The pointer is not resolved yet!
 
-  ValueLatticeElement &IV = ValueState[&I];
-
   if (SCCPSolver::isConstant(PtrVal)) {
     Constant *Ptr = getConstant(PtrVal, I.getOperand(0)->getType());
+    ValueLatticeElement &IV = ValueState[&I];
 
     // load null is undefined.
     if (isa<ConstantPointerNull>(Ptr)) {
@@ -1944,7 +1947,7 @@ void SCCPInstVisitor::handleCallOverdefined(CallBase &CB) {
         return markOverdefined(&CB); // Can't handle struct args.
       if (A.get()->getType()->isMetadataTy())
         continue;                    // Carried in CB, not allowed in Operands.
-      ValueLatticeElement State = getValueState(A);
+      const ValueLatticeElement &State = getValueState(A);
 
       if (State.isUnknownOrUndef())
         return; // Operands are not resolved yet.

nikic

LGTM apart from one question.

nikic · 2025-10-17T07:45:27Z

llvm/lib/Transforms/Utils/SCCPSolver.cpp

                    ValueLatticeElement::MergeOptions Opts = {
                        /*MayIncludeUndef=*/false, /*CheckWiden=*/false});

-  bool mergeInValue(Value *V, ValueLatticeElement MergeWithV,
+  bool mergeInValue(Value *V, const ValueLatticeElement &MergeWithV,


I'm not sure this one is always safe. For example in https://github.com/dtcxzyw/llvm-project/blob/6a67889a728f410074d12ccfdf46bdf54bf9f56b/llvm/lib/Transforms/Utils/SCCPSolver.cpp#L1630 MergeWithV is the result of getValueState(). In that case ValueState[V] is known to be initialized already due to the previous ValueState[&I].isOverdefined() check, but it seems easy to cause reference invalidation here.

I have removed the helper function to avoid implicit invalidations. Hopefully it makes the implementation cleaner.

fhahn

LGTM, thanks

nikic · 2025-10-17T20:07:21Z

llvm/lib/Transforms/Utils/SCCPSolver.cpp

        }
      } else
-        mergeInValue(&*AI,
+        mergeInValue(ValueState[&*AI], &*AI,


This looks potentially unsafe, is ValueState[&*AI] guaranteed to already exist?

nikic · 2025-10-17T20:08:03Z

llvm/lib/Transforms/Utils/SCCPSolver.cpp

+    const ValueLatticeElement &OpValState = getValueState(OpVal);
+    // Safety: ValueState[&I] doesn't invalidate OpValState since it is already
+    // in the map.
+    mergeInValue(ValueState[&I], &I, OpValState);


It's unfortunate that ValueState.at(&I) returns a const reference, otherwise we could use that to assert existence.

I added an assertion here.

[LVI][SCCP] Avoid copying ValueLatticeElement

6a67889

dtcxzyw requested a review from fhahn October 17, 2025 02:40

dtcxzyw requested a review from nikic as a code owner October 17, 2025 02:40

llvmbot added function-specialization llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Oct 17, 2025

nikic approved these changes Oct 17, 2025

View reviewed changes

fhahn approved these changes Oct 17, 2025

View reviewed changes

[SCCP] Avoid implicit reallocation

6b08eeb

nikic reviewed Oct 17, 2025

View reviewed changes

[SCCP] Address review comments

4ea449b

nikic approved these changes Oct 18, 2025

View reviewed changes

dtcxzyw merged commit 5d22980 into llvm:main Oct 18, 2025
10 checks passed

dtcxzyw deleted the perf/lattice-ref branch October 18, 2025 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LVI][SCCP] Avoid copying ValueLatticeElement #163901

[LVI][SCCP] Avoid copying ValueLatticeElement #163901

Uh oh!

dtcxzyw commented Oct 17, 2025 •

edited

Loading

Uh oh!

llvmbot commented Oct 17, 2025 •

edited

Loading

Uh oh!

nikic left a comment

Uh oh!

nikic Oct 17, 2025

Uh oh!

dtcxzyw Oct 17, 2025

Uh oh!

fhahn left a comment

Uh oh!

nikic Oct 17, 2025

Uh oh!

dtcxzyw Oct 18, 2025

Uh oh!

nikic Oct 17, 2025

Uh oh!

dtcxzyw Oct 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[LVI][SCCP] Avoid copying ValueLatticeElement #163901

[LVI][SCCP] Avoid copying ValueLatticeElement #163901

Uh oh!

Conversation

dtcxzyw commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

dtcxzyw Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

dtcxzyw Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

nikic Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

dtcxzyw Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dtcxzyw commented Oct 17, 2025 •

edited

Loading

llvmbot commented Oct 17, 2025 •

edited

Loading