[llvm] [SimplifyCFG] Don't limit the number of simultaneous forwards from switch condition (PR #95932)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 19 01:21:26 PDT 2024
zmodem wrote:
> To me, this limit doesn't make sense.
Looks like I added this in 4ab4a8e63a5a99face7d7b523529d3805385c93a
I think the point of the `Indexes.size()` check is explained by the "if that would mean that some of the destination blocks of the switch can be folded away" part of this comment:
https://github.com/llvm/llvm-project/blob/9cbedd925cba9e8ef76c50caa6d6ab4b0cc79c8f/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L5774-L5776
The point of the forwarding is to make *multiple blocks* produce the same value for the phi node (the switch condition), so that they can be folded into one block.
If we're forwarding the condition value for just a single block, that's just replacing a constant with a non-constant which seems counter-productive in general.
Looking at the example in #95919
> ```
> bb3: ; preds = %bb7, %bb2, %bb
> ...
> %i4 = phi i64 [ 1, %bb7 ], [ 1, %bb2 ], [ %arg1, %bb ]
> ```
> We can replace `[ 1, %bb2 ] with [ %arg1, %bb2 ]`.
What makes that beneficial is that the incoming value for `%bb` is already `%arg1`. That makes me think the right fix is rather to take that into account in `FindPHIForConditionForwarding`, something like:
```
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 4e2dc7f2b2f4..980dee7707ac 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5743,7 +5743,7 @@ static bool eliminateDeadSwitchCases(SwitchInst *SI, DomTreeUpdater *DTU,
/// by an unconditional branch), look at the phi node for BB in the successor
/// block and see if the incoming value is equal to CaseValue. If so, return
/// the phi node, and set PhiIndex to BB's index in the phi node.
-static PHINode *FindPHIForConditionForwarding(ConstantInt *CaseValue,
+static PHINode *FindPHIForConditionForwarding(Value *SwitchCondition, ConstantInt *CaseValue,
BasicBlock *BB, int *PhiIndex) {
if (BB->getFirstNonPHIOrDbg() != BB->getTerminator())
return nullptr; // BB must be empty to be a candidate for simplification.
@@ -5761,7 +5761,7 @@ static PHINode *FindPHIForConditionForwarding(ConstantInt *CaseValue,
assert(Idx >= 0 && "PHI has no entry for predecessor?");
Value *InValue = PHI.getIncomingValue(Idx);
- if (InValue != CaseValue)
+ if (InValue != CaseValue && InValue != SwitchCondition)
continue;
*PhiIndex = Idx;
@@ -5811,7 +5811,7 @@ static bool ForwardSwitchConditionToPHI(SwitchInst *SI) {
// Collect phi nodes that are indirectly using this switch's case constants.
int PhiIdx;
- if (auto *Phi = FindPHIForConditionForwarding(CaseValue, CaseDest, &PhiIdx))
+ if (auto *Phi = FindPHIForConditionForwarding(SI->getCondition(), CaseValue, CaseDest, &PhiIdx))
ForwardingNodes[Phi].push_back(PhiIdx);
}
```
https://github.com/llvm/llvm-project/pull/95932
More information about the llvm-commits
mailing list