[GlobalISel][IRTranslator] Fix some PHI bugs related to jump tables when optimizations are used.

The new switch lowering code that tries to generate jump tables and range checks
were tested at -O0 on arm64, but on -O3 the generic switch lowering code goes to
town on trying to generate optimized lowerings, e.g. multiple jump tables, range
checks etc. This exposed bugs in the way PHI nodes are handled because the CFG
looks even stranger after all of this is done.

llvm-svn: 364613

include/llvm/CodeGen/GlobalISel/IRTranslator.h

@@ -290,8 +290,7 @@ private:
   // Begin switch lowering functions.
   bool emitJumpTableHeader(SwitchCG::JumpTable &JT,
                            SwitchCG::JumpTableHeader &JTH,
-                           MachineBasicBlock *SwitchBB,
+                           MachineBasicBlock *HeaderBB);
-                           MachineIRBuilder &MIB);
   void emitJumpTable(SwitchCG::JumpTable &JT, MachineBasicBlock *MBB);
 
   void emitSwitchCase(SwitchCG::CaseBlock &CB, MachineBasicBlock *SwitchBB,
@@ -299,6 +298,7 @@ private:
 
   bool lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
                               MachineBasicBlock *SwitchMBB,
+                              MachineBasicBlock *CurMBB,
                               MachineBasicBlock *DefaultMBB,
                               MachineIRBuilder &MIB,
                               MachineFunction::iterator BBI,

lib/CodeGen/GlobalISel/IRTranslator.cpp

@@ -519,9 +519,10 @@ void IRTranslator::emitJumpTable(SwitchCG::JumpTable &JT,
 
 bool IRTranslator::emitJumpTableHeader(SwitchCG::JumpTable &JT,
                                        SwitchCG::JumpTableHeader &JTH,
-                                       MachineBasicBlock *SwitchBB,
+                                       MachineBasicBlock *HeaderBB) {
-                                       MachineIRBuilder &MIB) {
+  MachineIRBuilder MIB(*HeaderBB->getParent());
-  DebugLoc dl = MIB.getDebugLoc();
+  MIB.setMBB(*HeaderBB);
+  MIB.setDebugLoc(CurBuilder->getDebugLoc());
 
   const Value &SValue = *JTH.SValue;
   // Subtract the lowest switch case value from the value being switched on.
@@ -539,7 +540,7 @@ bool IRTranslator::emitJumpTableHeader(SwitchCG::JumpTable &JT,
   JT.Reg = Sub.getReg(0);
 
   if (JTH.OmitRangeCheck) {
-    if (JT.MBB != SwitchBB->getNextNode())
+    if (JT.MBB != HeaderBB->getNextNode())
       MIB.buildBr(*JT.MBB);
     return true;
   }
@@ -555,7 +556,7 @@ bool IRTranslator::emitJumpTableHeader(SwitchCG::JumpTable &JT,
   auto BrCond = MIB.buildBrCond(Cmp.getReg(0), *JT.Default);
 
   // Avoid emitting unnecessary branches to the next block.
-  if (JT.MBB != SwitchBB->getNextNode())
+  if (JT.MBB != HeaderBB->getNextNode())
     BrCond = MIB.buildBr(*JT.MBB);
   return true;
 }
@@ -618,8 +619,10 @@ void IRTranslator::emitSwitchCase(SwitchCG::CaseBlock &CB,
     addSuccessorWithProb(CB.ThisBB, CB.FalseBB, CB.FalseProb);
   CB.ThisBB->normalizeSuccProbs();
 
-  addMachineCFGPred({SwitchBB->getBasicBlock(), CB.FalseBB->getBasicBlock()},
+  //  if (SwitchBB->getBasicBlock() != CB.FalseBB->getBasicBlock())
-                    CB.ThisBB);
+    addMachineCFGPred({SwitchBB->getBasicBlock(), CB.FalseBB->getBasicBlock()},
+                      CB.ThisBB);
+
   // If the lhs block is the next block, invert the condition so that we can
   // fall through to the lhs instead of the rhs block.
   if (CB.TrueBB == CB.ThisBB->getNextNode()) {
@@ -636,6 +639,7 @@ void IRTranslator::emitSwitchCase(SwitchCG::CaseBlock &CB,
 
 bool IRTranslator::lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
                                           MachineBasicBlock *SwitchMBB,
+                                          MachineBasicBlock *CurMBB,
                                           MachineBasicBlock *DefaultMBB,
                                           MachineIRBuilder &MIB,
                                           MachineFunction::iterator BBI,
@@ -649,7 +653,6 @@ bool IRTranslator::lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
   JumpTableHeader *JTH = &SL->JTCases[I->JTCasesIndex].first;
   SwitchCG::JumpTable *JT = &SL->JTCases[I->JTCasesIndex].second;
   BranchProbability DefaultProb = W.DefaultProb;
-  MachineBasicBlock *CurMBB = W.MBB;
 
   // The jump block hasn't been inserted yet; insert it here.
   MachineBasicBlock *JumpMBB = JT->MBB;
@@ -659,7 +662,7 @@ bool IRTranslator::lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
   // to keep track of it as a machine predecessor to the default block,
   // otherwise we lose the phi edges.
   addMachineCFGPred({SwitchMBB->getBasicBlock(), DefaultMBB->getBasicBlock()},
-                    SwitchMBB);
+                    CurMBB);
   addMachineCFGPred({SwitchMBB->getBasicBlock(), DefaultMBB->getBasicBlock()},
                     JumpMBB);
 
@@ -677,7 +680,10 @@ bool IRTranslator::lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
       FallthroughProb -= DefaultProb / 2;
       JumpMBB->setSuccProbability(SI, DefaultProb / 2);
       JumpMBB->normalizeSuccProbs();
-      break;
+    } else {
+      // Also record edges from the jump table block to it's successors.
+      addMachineCFGPred({SwitchMBB->getBasicBlock(), (*SI)->getBasicBlock()},
+                        JumpMBB);
     }
   }
 
@@ -697,7 +703,7 @@ bool IRTranslator::lowerJumpTableWorkItem(SwitchCG::SwitchWorkListItem W,
 
   // If we're in the right place, emit the jump table header right now.
   if (CurMBB == SwitchMBB) {
-    if (!emitJumpTableHeader(*JT, *JTH, SwitchMBB, MIB))
+    if (!emitJumpTableHeader(*JT, *JTH, CurMBB))
       return false;
     JTH->Emitted = true;
   }
@@ -801,7 +807,7 @@ bool IRTranslator::lowerSwitchWorkItem(SwitchCG::SwitchWorkListItem W,
       return false; // Bit tests currently unimplemented.
     }
     case CC_JumpTable: {
-      if (!lowerJumpTableWorkItem(W, SwitchMBB, DefaultMBB, MIB, BBI,
+      if (!lowerJumpTableWorkItem(W, SwitchMBB, CurMBB, DefaultMBB, MIB, BBI,
                                   UnhandledProbs, I, Fallthrough,
                                   FallthroughUnreachable)) {
         LLVM_DEBUG(dbgs() << "Failed to lower jump table");
@@ -2030,6 +2036,7 @@ void IRTranslator::finishPendingPhis() {
   for (auto &Phi : PendingPHIs) {
     const PHINode *PI = Phi.first;
     ArrayRef<MachineInstr *> ComponentPHIs = Phi.second;
+    MachineBasicBlock *PhiMBB = ComponentPHIs[0]->getParent();
     EntryBuilder->setDebugLoc(PI->getDebugLoc());
 #ifndef NDEBUG
     Verifier.setCurrentInst(PI);
@@ -2040,7 +2047,7 @@ void IRTranslator::finishPendingPhis() {
       auto IRPred = PI->getIncomingBlock(i);
       ArrayRef<Register> ValRegs = getOrCreateVRegs(*PI->getIncomingValue(i));
       for (auto Pred : getMachinePredBBs({IRPred, PI->getParent()})) {
-        if (SeenPreds.count(Pred))
+        if (SeenPreds.count(Pred) || !PhiMBB->isPredecessor(Pred))
           continue;
         SeenPreds.insert(Pred);
         for (unsigned j = 0; j < ValRegs.size(); ++j) {
@@ -2147,8 +2154,13 @@ bool IRTranslator::translate(const Constant &C, Register Reg) {
 }
 
 void IRTranslator::finalizeBasicBlock() {
-  for (auto &JTCase : SL->JTCases)
+  for (auto &JTCase : SL->JTCases) {
+    // Emit header first, if it wasn't already emitted.
+    if (!JTCase.first.Emitted)
+      emitJumpTableHeader(JTCase.second, JTCase.first, JTCase.first.HeaderBB);
+
     emitJumpTable(JTCase.second, JTCase.second.MBB);
+  }
   SL->JTCases.clear();
 }