[llvm] r316293 - [SimplifyCFG] try harder to forward switch condition to phi (PR34471)

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Sun Oct 22 09:51:03 2017
New Revision: 316293

URL: http://llvm.org/viewvc/llvm-project?rev=316293&view=rev
Log:
[SimplifyCFG] try harder to forward switch condition to phi (PR34471)

The missed canonicalization/optimization in the motivating test from PR34471 leads to very different codegen:

  int switcher(int x) {
      switch(x) {
      case 17: return 17;
      case 19: return 19;
      case 42: return 42;
      default: break;
      }
      return 0;
    }

  int comparator(int x) {
    if (x == 17) return 17;
    if (x == 19) return 19;
    if (x == 42) return 42;
    return 0;
  }

For the first example, we use a bit-test optimization to avoid a series of compare-and-branch:
https://godbolt.org/g/BivDsw

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


Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=316293&r1=316292&r2=316293&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Sun Oct 22 09:51:03 2017
@@ -4450,16 +4450,46 @@ static PHINode *FindPHIForConditionForwa
 static bool ForwardSwitchConditionToPHI(SwitchInst *SI) {
   typedef DenseMap<PHINode *, SmallVector<int, 4>> ForwardingNodesMap;
   ForwardingNodesMap ForwardingNodes;
-
+  BasicBlock *SwitchBlock = SI->getParent();
+  bool Changed = false;
   for (auto &Case : SI->cases()) {
     ConstantInt *CaseValue = Case.getCaseValue();
     BasicBlock *CaseDest = Case.getCaseSuccessor();
+
+    // Replace phi operands in successor blocks that are using the constant case
+    // value rather than the switch condition variable:
+    //   switchbb:
+    //   switch i32 %x, label %default [
+    //     i32 17, label %succ
+    //   ...
+    //   succ:
+    //     %r = phi i32 ... [ 17, %switchbb ] ...
+    // -->
+    //     %r = phi i32 ... [ %x, %switchbb ] ...
+
+    for (Instruction &InstInCaseDest : *CaseDest) {
+      auto *Phi = dyn_cast<PHINode>(&InstInCaseDest);
+      if (!Phi) break;
+
+      // This only works if there is exactly 1 incoming edge from the switch to
+      // a phi. If there is >1, that means multiple cases of the switch map to 1
+      // value in the phi, and that phi value is not the switch condition. Thus,
+      // this transform would not make sense (the phi would be invalid because
+      // a phi can't have different incoming values from the same block).
+      int SwitchBBIdx = Phi->getBasicBlockIndex(SwitchBlock);
+      if (Phi->getIncomingValue(SwitchBBIdx) == CaseValue &&
+          count(Phi->blocks(), SwitchBlock) == 1) {
+        Phi->setIncomingValue(SwitchBBIdx, SI->getCondition());
+        Changed = true;
+      }
+    }
+
+    // Collect phi nodes that are indirectly using this switch's case constants.
     int PhiIdx;
     if (auto *Phi = FindPHIForConditionForwarding(CaseValue, CaseDest, &PhiIdx))
       ForwardingNodes[Phi].push_back(PhiIdx);
   }
 
-  bool Changed = false;
   for (auto &ForwardingNode : ForwardingNodes) {
     PHINode *Phi = ForwardingNode.first;
     SmallVectorImpl<int> &Indexes = ForwardingNode.second;

Modified: llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll?rev=316293&r1=316292&r2=316293&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll Sun Oct 22 09:51:03 2017
@@ -50,17 +50,13 @@ define i32 @PR34471(i32 %x) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    switch i32 [[X:%.*]], label [[ELSE3:%.*]] [
 ; CHECK-NEXT:    i32 17, label [[RETURN:%.*]]
-; CHECK-NEXT:    i32 19, label [[IF19:%.*]]
-; CHECK-NEXT:    i32 42, label [[IF42:%.*]]
+; CHECK-NEXT:    i32 19, label [[RETURN]]
+; CHECK-NEXT:    i32 42, label [[RETURN]]
 ; CHECK-NEXT:    ]
-; CHECK:       if19:
-; CHECK-NEXT:    br label [[RETURN]]
-; CHECK:       if42:
-; CHECK-NEXT:    br label [[RETURN]]
 ; CHECK:       else3:
 ; CHECK-NEXT:    br label [[RETURN]]
 ; CHECK:       return:
-; CHECK-NEXT:    [[R:%.*]] = phi i32 [ [[X]], [[IF19]] ], [ [[X]], [[IF42]] ], [ 0, [[ELSE3]] ], [ 17, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[R:%.*]] = phi i32 [ 0, [[ELSE3]] ], [ [[X]], [[ENTRY:%.*]] ], [ [[X]], [[ENTRY]] ], [ [[X]], [[ENTRY]] ]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
 entry: