Skip to content

Conversation

@nikic
Copy link
Contributor

@nikic nikic commented Jun 19, 2025

As far as I can tell, this set is pointless: It just represents whether the target block has multiple predecessors, and the way it is constructed and queried, we're not even reducing the number of getSinglePredecessor() calls or something like that.

As far as I can tell, this set is pointless: It just represents
whether the target block has multiple predecessors, and the way
it is constructed and queried, we're not even reducing the number
of getSinglePredecessor() calls or something like that.
@nikic nikic requested review from dtcxzyw and fhahn June 19, 2025 15:17
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Nikita Popov (nikic)

Changes

As far as I can tell, this set is pointless: It just represents whether the target block has multiple predecessors, and the way it is constructed and queried, we're not even reducing the number of getSinglePredecessor() calls or something like that.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/PredicateInfo.cpp (+1-9)
diff --git a/llvm/lib/Transforms/Utils/PredicateInfo.cpp b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
index 4c87babbfb6f3..4a0faab00ccaf 100644
--- a/llvm/lib/Transforms/Utils/PredicateInfo.cpp
+++ b/llvm/lib/Transforms/Utils/PredicateInfo.cpp
@@ -253,10 +253,6 @@ class PredicateInfoBuilder {
   // whether it returned a valid result.
   DenseMap<Value *, unsigned int> ValueInfoNums;
 
-  // The set of edges along which we can only handle phi uses, due to critical
-  // edges.
-  DenseSet<std::pair<BasicBlock *, BasicBlock *>> EdgeUsesOnly;
-
   ValueInfo &getOrCreateValueInfo(Value *);
   const ValueInfo &getValueInfo(Value *) const;
 
@@ -459,8 +455,6 @@ void PredicateInfoBuilder::processBranch(
           PredicateBase *PB =
               new PredicateBranch(V, BranchBB, Succ, Cond, TakenEdge);
           addInfoFor(OpsToRename, V, PB);
-          if (!Succ->getSinglePredecessor())
-            EdgeUsesOnly.insert({BranchBB, Succ});
         }
       }
     }
@@ -487,8 +481,6 @@ void PredicateInfoBuilder::processSwitch(
       PredicateSwitch *PS = new PredicateSwitch(
           Op, SI->getParent(), TargetBlock, C.getCaseValue(), SI);
       addInfoFor(OpsToRename, Op, PS);
-      if (!TargetBlock->getSinglePredecessor())
-        EdgeUsesOnly.insert({BranchBB, TargetBlock});
     }
   }
 }
@@ -637,7 +629,7 @@ void PredicateInfoBuilder::renameUses(SmallVectorImpl<Value *> &OpsToRename) {
         // block, and handle it specially. We know that it goes last, and only
         // dominate phi uses.
         auto BlockEdge = getBlockEdge(PossibleCopy);
-        if (EdgeUsesOnly.count(BlockEdge)) {
+        if (!BlockEdge.second->getSinglePredecessor()) {
           VD.LocalNum = LN_Last;
           auto *DomNode = DT.getNode(BlockEdge.first);
           if (DomNode) {

Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

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

LG.

@nikic nikic merged commit d196124 into llvm:main Jun 20, 2025
9 checks passed
@nikic nikic deleted the predicateinfo-edge-only branch June 20, 2025 07:16
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.

3 participants