[llvm] a794b62 - [SCCPSolver] Move helper functions inside SCCPSolver (NFC).

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 09:42:09 PST 2023


Author: Florian Hahn
Date: 2023-01-23T17:41:33Z
New Revision: a794b62c0cee998d438b31e79d2c39c92d1fec36

URL: https://github.com/llvm/llvm-project/commit/a794b62c0cee998d438b31e79d2c39c92d1fec36
DIFF: https://github.com/llvm/llvm-project/commit/a794b62c0cee998d438b31e79d2c39c92d1fec36.diff

LOG: [SCCPSolver] Move helper functions inside SCCPSolver (NFC).

This patch moves a couple of helper functions from the global llvm::
namespace into the SCCPSolver class. This reduces the need for separate
SCCPSolver arguments and also limits the scope of those functions that
have quite generic names.

(The remaining isConstant and isOverdefined should ideally be removed)

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D142370

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/SCCPSolver.h
    llvm/lib/Transforms/IPO/SCCP.cpp
    llvm/lib/Transforms/Scalar/SCCP.cpp
    llvm/lib/Transforms/Utils/SCCPSolver.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/SCCPSolver.h b/llvm/include/llvm/Transforms/Utils/SCCPSolver.h
index d5c99afafe30..633052f1c15e 100644
--- a/llvm/include/llvm/Transforms/Utils/SCCPSolver.h
+++ b/llvm/include/llvm/Transforms/Utils/SCCPSolver.h
@@ -187,26 +187,29 @@ class SCCPSolver {
 
   void visit(Instruction *I);
   void visitCall(CallInst &I);
-};
 
-//===----------------------------------------------------------------------===//
-//
-/// Helper functions used by the SCCP and IPSCCP passes.
-//
-bool isConstant(const ValueLatticeElement &LV);
+  bool simplifyInstsInBlock(BasicBlock &BB,
+                            SmallPtrSetImpl<Value *> &InsertedValues,
+                            Statistic &InstRemovedStat,
+                            Statistic &InstReplacedStat);
 
-bool isOverdefined(const ValueLatticeElement &LV);
+  bool removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
+                              BasicBlock *&NewUnreachableBB) const;
 
-bool simplifyInstsInBlock(SCCPSolver &Solver, BasicBlock &BB,
-                          SmallPtrSetImpl<Value *> &InsertedValues,
-                          Statistic &InstRemovedStat,
-                          Statistic &InstReplacedStat);
+  bool tryToReplaceWithConstant(Value *V);
 
-bool tryToReplaceWithConstant(SCCPSolver &Solver, Value *V);
+  // Helper to check if \p LV is either a constant or a constant
+  // range with a single element. This should cover exactly the same cases as
+  // the old ValueLatticeElement::isConstant() and is intended to be used in the
+  // transition to ValueLatticeElement.
+  static bool isConstant(const ValueLatticeElement &LV);
 
-bool removeNonFeasibleEdges(const llvm::SCCPSolver &Solver, BasicBlock *BB,
-                            DomTreeUpdater &DTU,
-                            BasicBlock *&NewUnreachableBB);
+  // Helper to check if \p LV is either overdefined or a constant range with
+  // more than a single element. This should cover exactly the same cases as the
+  // old ValueLatticeElement::isOverdefined() and is intended to be used in the
+  // transition to ValueLatticeElement.
+  static bool isOverdefined(const ValueLatticeElement &LV);
+};
 } // namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_SCCPSOLVER_H

diff  --git a/llvm/lib/Transforms/IPO/SCCP.cpp b/llvm/lib/Transforms/IPO/SCCP.cpp
index b5df5a1ae1c2..5c1582ddfdae 100644
--- a/llvm/lib/Transforms/IPO/SCCP.cpp
+++ b/llvm/lib/Transforms/IPO/SCCP.cpp
@@ -77,7 +77,7 @@ static void findReturnsToZap(Function &F,
                if (U->getType()->isStructTy()) {
                  return all_of(Solver.getStructLatticeValueFor(U),
                                [](const ValueLatticeElement &LV) {
-                                 return !isOverdefined(LV);
+                                 return !SCCPSolver::isOverdefined(LV);
                                });
                }
 
@@ -88,7 +88,7 @@ static void findReturnsToZap(Function &F,
                    return true;
                }
 
-               return !isOverdefined(Solver.getLatticeValueFor(U));
+               return !SCCPSolver::isOverdefined(Solver.getLatticeValueFor(U));
              }) &&
       "We can only zap functions where all live users have a concrete value");
 
@@ -173,7 +173,7 @@ static bool runIPSCCP(
     if (Solver.isBlockExecutable(&F.front())) {
       bool ReplacedPointerArg = false;
       for (Argument &Arg : F.args()) {
-        if (!Arg.use_empty() && tryToReplaceWithConstant(Solver, &Arg)) {
+        if (!Arg.use_empty() && Solver.tryToReplaceWithConstant(&Arg)) {
           ReplacedPointerArg |= Arg.getType()->isPointerTy();
           ++NumArgsElimed;
         }
@@ -219,8 +219,8 @@ static bool runIPSCCP(
         continue;
       }
 
-      MadeChanges |= simplifyInstsInBlock(Solver, BB, InsertedValues,
-                                          NumInstRemoved, NumInstReplaced);
+      MadeChanges |= Solver.simplifyInstsInBlock(
+          BB, InsertedValues, NumInstRemoved, NumInstReplaced);
     }
 
     DomTreeUpdater DTU = IsFuncSpecEnabled && Specializer.isClonedFunction(&F)
@@ -241,7 +241,7 @@ static bool runIPSCCP(
 
     BasicBlock *NewUnreachableBB = nullptr;
     for (BasicBlock &BB : F)
-      MadeChanges |= removeNonFeasibleEdges(Solver, &BB, DTU, NewUnreachableBB);
+      MadeChanges |= Solver.removeNonFeasibleEdges(&BB, DTU, NewUnreachableBB);
 
     for (BasicBlock *DeadBB : BlocksToErase)
       if (!DeadBB->hasAddressTaken())
@@ -315,7 +315,7 @@ static bool runIPSCCP(
     }
     if (F->getReturnType()->isVoidTy())
       continue;
-    if (isConstant(ReturnValue) || ReturnValue.isUnknownOrUndef())
+    if (SCCPSolver::isConstant(ReturnValue) || ReturnValue.isUnknownOrUndef())
       findReturnsToZap(*F, ReturnsToZap, Solver);
   }
 
@@ -361,7 +361,7 @@ static bool runIPSCCP(
   // delete the global and any stores that remain to it.
   for (const auto &I : make_early_inc_range(Solver.getTrackedGlobals())) {
     GlobalVariable *GV = I.first;
-    if (isOverdefined(I.second))
+    if (SCCPSolver::isOverdefined(I.second))
       continue;
     LLVM_DEBUG(dbgs() << "Found that GV '" << GV->getName()
                       << "' is constant!\n");

diff  --git a/llvm/lib/Transforms/Scalar/SCCP.cpp b/llvm/lib/Transforms/Scalar/SCCP.cpp
index fd7d4820bf5c..7b396c6ee074 100644
--- a/llvm/lib/Transforms/Scalar/SCCP.cpp
+++ b/llvm/lib/Transforms/Scalar/SCCP.cpp
@@ -104,8 +104,8 @@ static bool runSCCP(Function &F, const DataLayout &DL,
       continue;
     }
 
-    MadeChanges |= simplifyInstsInBlock(Solver, BB, InsertedValues,
-                                        NumInstRemoved, NumInstReplaced);
+    MadeChanges |= Solver.simplifyInstsInBlock(BB, InsertedValues,
+                                               NumInstRemoved, NumInstReplaced);
   }
 
   // Remove unreachable blocks and non-feasible edges.
@@ -115,7 +115,7 @@ static bool runSCCP(Function &F, const DataLayout &DL,
 
   BasicBlock *NewUnreachableBB = nullptr;
   for (BasicBlock &BB : F)
-    MadeChanges |= removeNonFeasibleEdges(Solver, &BB, DTU, NewUnreachableBB);
+    MadeChanges |= Solver.removeNonFeasibleEdges(&BB, DTU, NewUnreachableBB);
 
   for (BasicBlock *DeadBB : BlocksToErase)
     if (!DeadBB->hasAddressTaken())

diff  --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index a92ba67503a2..8d03a0d8a2c4 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -43,21 +43,13 @@ static ValueLatticeElement::MergeOptions getMaxWidenStepsOpts() {
 
 namespace llvm {
 
-// Helper to check if \p LV is either a constant or a constant
-// range with a single element. This should cover exactly the same cases as the
-// old ValueLatticeElement::isConstant() and is intended to be used in the
-// transition to ValueLatticeElement.
-bool isConstant(const ValueLatticeElement &LV) {
+bool SCCPSolver::isConstant(const ValueLatticeElement &LV) {
   return LV.isConstant() ||
          (LV.isConstantRange() && LV.getConstantRange().isSingleElement());
 }
 
-// Helper to check if \p LV is either overdefined or a constant range with more
-// than a single element. This should cover exactly the same cases as the old
-// ValueLatticeElement::isOverdefined() and is intended to be used in the
-// transition to ValueLatticeElement.
-bool isOverdefined(const ValueLatticeElement &LV) {
-  return !LV.isUnknownOrUndef() && !isConstant(LV);
+bool SCCPSolver::isOverdefined(const ValueLatticeElement &LV) {
+  return !LV.isUnknownOrUndef() && !SCCPSolver::isConstant(LV);
 }
 
 static bool canRemoveInstruction(Instruction *I) {
@@ -72,28 +64,28 @@ static bool canRemoveInstruction(Instruction *I) {
   return isa<LoadInst>(I);
 }
 
-bool tryToReplaceWithConstant(SCCPSolver &Solver, Value *V) {
+bool SCCPSolver::tryToReplaceWithConstant(Value *V) {
   Constant *Const = nullptr;
   if (V->getType()->isStructTy()) {
-    std::vector<ValueLatticeElement> IVs = Solver.getStructLatticeValueFor(V);
+    std::vector<ValueLatticeElement> IVs = getStructLatticeValueFor(V);
     if (llvm::any_of(IVs, isOverdefined))
       return false;
     std::vector<Constant *> ConstVals;
     auto *ST = cast<StructType>(V->getType());
     for (unsigned i = 0, e = ST->getNumElements(); i != e; ++i) {
       ValueLatticeElement V = IVs[i];
-      ConstVals.push_back(isConstant(V)
-                              ? Solver.getConstant(V)
+      ConstVals.push_back(SCCPSolver::isConstant(V)
+                              ? getConstant(V)
                               : UndefValue::get(ST->getElementType(i)));
     }
     Const = ConstantStruct::get(ST, ConstVals);
   } else {
-    const ValueLatticeElement &IV = Solver.getLatticeValueFor(V);
+    const ValueLatticeElement &IV = getLatticeValueFor(V);
     if (isOverdefined(IV))
       return false;
 
-    Const =
-        isConstant(IV) ? Solver.getConstant(IV) : UndefValue::get(V->getType());
+    Const = SCCPSolver::isConstant(IV) ? getConstant(IV)
+                                       : UndefValue::get(V->getType());
   }
   assert(Const && "Constant is nullptr here!");
 
@@ -109,7 +101,7 @@ bool tryToReplaceWithConstant(SCCPSolver &Solver, Value *V) {
 
     // Don't zap returns of the callee
     if (F)
-      Solver.addToMustPreserveReturnsInFunctions(F);
+      addToMustPreserveReturnsInFunctions(F);
 
     LLVM_DEBUG(dbgs() << "  Can\'t treat the result of call " << *CB
                       << " as a constant\n");
@@ -186,21 +178,21 @@ static bool replaceSignedInst(SCCPSolver &Solver,
   return true;
 }
 
-bool simplifyInstsInBlock(SCCPSolver &Solver, BasicBlock &BB,
-                          SmallPtrSetImpl<Value *> &InsertedValues,
-                          Statistic &InstRemovedStat,
-                          Statistic &InstReplacedStat) {
+bool SCCPSolver::simplifyInstsInBlock(BasicBlock &BB,
+                                      SmallPtrSetImpl<Value *> &InsertedValues,
+                                      Statistic &InstRemovedStat,
+                                      Statistic &InstReplacedStat) {
   bool MadeChanges = false;
   for (Instruction &Inst : make_early_inc_range(BB)) {
     if (Inst.getType()->isVoidTy())
       continue;
-    if (tryToReplaceWithConstant(Solver, &Inst)) {
+    if (tryToReplaceWithConstant(&Inst)) {
       if (canRemoveInstruction(&Inst))
         Inst.eraseFromParent();
 
       MadeChanges = true;
       ++InstRemovedStat;
-    } else if (replaceSignedInst(Solver, InsertedValues, Inst)) {
+    } else if (replaceSignedInst(*this, InsertedValues, Inst)) {
       MadeChanges = true;
       ++InstReplacedStat;
     }
@@ -208,13 +200,12 @@ bool simplifyInstsInBlock(SCCPSolver &Solver, BasicBlock &BB,
   return MadeChanges;
 }
 
-bool removeNonFeasibleEdges(const SCCPSolver &Solver, BasicBlock *BB,
-                            DomTreeUpdater &DTU,
-                            BasicBlock *&NewUnreachableBB) {
+bool SCCPSolver::removeNonFeasibleEdges(BasicBlock *BB, DomTreeUpdater &DTU,
+                                        BasicBlock *&NewUnreachableBB) const {
   SmallPtrSet<BasicBlock *, 8> FeasibleSuccessors;
   bool HasNonFeasibleEdges = false;
   for (BasicBlock *Succ : successors(BB)) {
-    if (Solver.isEdgeFeasible(BB, Succ))
+    if (isEdgeFeasible(BB, Succ))
       FeasibleSuccessors.insert(Succ);
     else
       HasNonFeasibleEdges = true;
@@ -774,7 +765,7 @@ bool SCCPInstVisitor::isStructLatticeConstant(Function *F, StructType *STy) {
     const auto &It = TrackedMultipleRetVals.find(std::make_pair(F, i));
     assert(It != TrackedMultipleRetVals.end());
     ValueLatticeElement LV = It->second;
-    if (!isConstant(LV))
+    if (!SCCPSolver::isConstant(LV))
       return false;
   }
   return true;
@@ -1184,7 +1175,7 @@ void SCCPInstVisitor::visitInsertValueInst(InsertValueInst &IVI) {
 
   // resolvedUndefsIn might mark I as overdefined. Bail out, even if we would
   // discover a concrete value later.
-  if (isOverdefined(ValueState[&IVI]))
+  if (SCCPSolver::isOverdefined(ValueState[&IVI]))
     return (void)markOverdefined(&IVI);
 
   // If this has more than one index, we can't handle it, drive all results to
@@ -1255,14 +1246,14 @@ void SCCPInstVisitor::visitUnaryOperator(Instruction &I) {
   ValueLatticeElement &IV = ValueState[&I];
   // resolvedUndefsIn might mark I as overdefined. Bail out, even if we would
   // discover a concrete value later.
-  if (isOverdefined(IV))
+  if (SCCPSolver::isOverdefined(IV))
     return (void)markOverdefined(&I);
 
   // If something is unknown/undef, wait for it to resolve.
   if (V0State.isUnknownOrUndef())
     return;
 
-  if (isConstant(V0State))
+  if (SCCPSolver::isConstant(V0State))
     if (Constant *C = ConstantFoldUnaryOpOperand(I.getOpcode(),
                                                  getConstant(V0State), DL))
       return (void)markConstant(IV, &I, C);
@@ -1289,8 +1280,10 @@ void SCCPInstVisitor::visitBinaryOperator(Instruction &I) {
   // If either of the operands is a constant, try to fold it to a constant.
   // TODO: Use information from notconstant better.
   if ((V1State.isConstant() || V2State.isConstant())) {
-    Value *V1 = isConstant(V1State) ? getConstant(V1State) : I.getOperand(0);
-    Value *V2 = isConstant(V2State) ? getConstant(V2State) : I.getOperand(1);
+    Value *V1 = SCCPSolver::isConstant(V1State) ? getConstant(V1State)
+                                                : I.getOperand(0);
+    Value *V2 = SCCPSolver::isConstant(V2State) ? getConstant(V2State)
+                                                : I.getOperand(1);
     Value *R = simplifyBinOp(I.getOpcode(), V1, V2, SimplifyQuery(DL));
     auto *C = dyn_cast_or_null<Constant>(R);
     if (C) {
@@ -1324,7 +1317,7 @@ void SCCPInstVisitor::visitBinaryOperator(Instruction &I) {
 void SCCPInstVisitor::visitCmpInst(CmpInst &I) {
   // Do not cache this lookup, getValueState calls later in the function might
   // invalidate the reference.
-  if (isOverdefined(ValueState[&I]))
+  if (SCCPSolver::isOverdefined(ValueState[&I]))
     return (void)markOverdefined(&I);
 
   Value *Op1 = I.getOperand(0);
@@ -1345,7 +1338,7 @@ void SCCPInstVisitor::visitCmpInst(CmpInst &I) {
 
   // If operands are still unknown, wait for it to resolve.
   if ((V1State.isUnknownOrUndef() || V2State.isUnknownOrUndef()) &&
-      !isConstant(ValueState[&I]))
+      !SCCPSolver::isConstant(ValueState[&I]))
     return;
 
   markOverdefined(&I);
@@ -1354,7 +1347,7 @@ void SCCPInstVisitor::visitCmpInst(CmpInst &I) {
 // Handle getelementptr instructions.  If all operands are constants then we
 // can turn this into a getelementptr ConstantExpr.
 void SCCPInstVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
-  if (isOverdefined(ValueState[&I]))
+  if (SCCPSolver::isOverdefined(ValueState[&I]))
     return (void)markOverdefined(&I);
 
   SmallVector<Constant *, 8> Operands;
@@ -1365,7 +1358,7 @@ void SCCPInstVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
     if (State.isUnknownOrUndef())
       return; // Operands are not resolved yet.
 
-    if (isOverdefined(State))
+    if (SCCPSolver::isOverdefined(State))
       return (void)markOverdefined(&I);
 
     if (Constant *C = getConstant(State)) {
@@ -1433,7 +1426,7 @@ void SCCPInstVisitor::visitLoadInst(LoadInst &I) {
 
   ValueLatticeElement &IV = ValueState[&I];
 
-  if (isConstant(PtrVal)) {
+  if (SCCPSolver::isConstant(PtrVal)) {
     Constant *Ptr = getConstant(PtrVal);
 
     // load null is undefined.
@@ -1494,13 +1487,13 @@ void SCCPInstVisitor::handleCallOverdefined(CallBase &CB) {
 
       if (State.isUnknownOrUndef())
         return; // Operands are not resolved yet.
-      if (isOverdefined(State))
+      if (SCCPSolver::isOverdefined(State))
         return (void)markOverdefined(&CB);
-      assert(isConstant(State) && "Unknown state!");
+      assert(SCCPSolver::isConstant(State) && "Unknown state!");
       Operands.push_back(getConstant(State));
     }
 
-    if (isOverdefined(getValueState(&CB)))
+    if (SCCPSolver::isOverdefined(getValueState(&CB)))
       return (void)markOverdefined(&CB);
 
     // If we can constant fold this, mark the result of the call as a


        


More information about the llvm-commits mailing list