[llvm] r336659 - [LowerSwitch] Fixed faulty PHI nodes

[Karl-Johan Karlsson via llvm-commits]

Author: karka
Date: Tue Jul 10 05:06:16 2018
New Revision: 336659

URL: http://llvm.org/viewvc/llvm-project?rev=336659&view=rev
Log:
[LowerSwitch] Fixed faulty PHI nodes

Summary:
Fixed two cases of where PHI nodes need to be updated by lowerswitch.

When lowerswitch find out that the switch default branch is not
reachable it remove the old default and replace it with the most
popular block from the cases, but it forget to update the PHI
nodes in the default block.

The PHI nodes also need to be updated when the switch is replaced
with a single branch.

Reviewers: hans, reames, arsenm

Reviewed By: arsenm

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

Modified:
    llvm/trunk/lib/Transforms/Utils/LowerSwitch.cpp
    llvm/trunk/test/Transforms/Util/lowerswitch.ll

Modified: llvm/trunk/lib/Transforms/Utils/LowerSwitch.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LowerSwitch.cpp?rev=336659&r1=336658&r2=336659&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/LowerSwitch.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/LowerSwitch.cpp Tue Jul 10 05:06:16 2018
@@ -74,7 +74,7 @@ namespace {
 
     LowerSwitch() : FunctionPass(ID) {
       initializeLowerSwitchPass(*PassRegistry::getPassRegistry());
-    } 
+    }
 
     bool runOnFunction(Function &F) override;
 
@@ -327,7 +327,7 @@ BasicBlock* LowerSwitch::newLeafBlock(Ca
     } else if (Leaf.Low->isZero()) {
       // Val >= 0 && Val <= Hi --> Val <=u Hi
       Comp = new ICmpInst(*NewLeaf, ICmpInst::ICMP_ULE, Val, Leaf.High,
-                          "SwitchLeaf");      
+                          "SwitchLeaf");
     } else {
       // Emit V-Lo <=u Hi-Lo
       Constant* NegLo = ConstantExpr::getNeg(Leaf.Low);
@@ -354,7 +354,7 @@ BasicBlock* LowerSwitch::newLeafBlock(Ca
     for (uint64_t j = 0; j < Range; ++j) {
       PN->removeIncomingValue(OrigBlock);
     }
-    
+
     int BlockIdx = PN->getBasicBlockIndex(OrigBlock);
     assert(BlockIdx != -1 && "Switch didn't go to this successor??");
     PN->setIncomingBlock((unsigned)BlockIdx, NewLeaf);
@@ -495,6 +495,10 @@ void LowerSwitch::processSwitchInst(Swit
     }
 #endif
 
+    // As the default block in the switch is unreachable, update the PHI nodes
+    // (remove the entry to the default block) to reflect this.
+    Default->removePredecessor(OrigBlock);
+
     // Use the most popular block as the new default, reducing the number of
     // cases.
     assert(MaxPop > 0 && PopSucc);
@@ -508,6 +512,10 @@ void LowerSwitch::processSwitchInst(Swit
     if (Cases.empty()) {
       BranchInst::Create(Default, CurBlock);
       SI->eraseFromParent();
+      // As all the cases have been replaced with a single branch, only keep
+      // one entry in the PHI nodes.
+      for (unsigned I = 0 ; I < (MaxPop - 1) ; ++I)
+        PopSucc->removePredecessor(OrigBlock);
       return;
     }
   }

Modified: llvm/trunk/test/Transforms/Util/lowerswitch.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Util/lowerswitch.ll?rev=336659&r1=336658&r2=336659&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Util/lowerswitch.ll (original)
+++ llvm/trunk/test/Transforms/Util/lowerswitch.ll Tue Jul 10 05:06:16 2018
@@ -242,3 +242,51 @@ cleanup:
 unreachable:                                      ; preds = %cleanup
   unreachable
 }
+
+; Test that the PHI node in cleanup17 is removed as the switch default block is
+; not reachable.
+define void @test4() {
+; CHECK-LABEL: @test4
+entry:
+  switch i32 undef, label %cleanup17 [
+    i32 0, label %return
+    i32 9, label %return
+  ]
+
+cleanup17:
+; CHECK: cleanup17:
+; CHECK-NOT: phi i16 [ undef, %entry ]
+; CHECK: return:
+
+  %retval.4 = phi i16 [ undef, %entry ]
+  unreachable
+
+return:
+  ret void
+}
+
+; Test that the PHI node in for.inc is updated correctly as the switch is
+; replaced with a single branch to for.inc
+define void @test5() {
+; CHECK-LABEL: @test5
+entry:
+  br i1 undef, label %cleanup10, label %cleanup10.thread
+
+cleanup10.thread:
+  br label %for.inc
+
+cleanup10:
+  switch i32 undef, label %unreachable [
+    i32 0, label %for.inc
+    i32 4, label %for.inc
+  ]
+
+for.inc:
+; CHECK: for.inc:
+; CHECK-NEXT: phi i16 [ 0, %cleanup10.thread ], [ undef, %cleanup10 ]
+%0 = phi i16 [ undef, %cleanup10 ], [ 0, %cleanup10.thread ], [ undef, %cleanup10 ]
+  unreachable
+
+unreachable:
+  unreachable
+}