[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