From 2622f4622c36ec9924fb908085154ffdd7174aff Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Thu, 30 Sep 2010 19:44:31 +0000 Subject: [PATCH] When isel is emitting instructions for an x86 target without CMOV, the CFG is edited during emission. If the basic block ends in a switch that gets lowered to a jump table, any phis at the default edge were getting updated wrong. The jump table data structure keeps a pointer to the header blocks that wasn't getting updated after the MBB is split. This bug was exposed on 32-bit Linux when disabling critical edge splitting in codegen prepare. The fix is to uipdate stale MBB pointers whenever a block is split during emission. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@115191 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../SelectionDAG/SelectionDAGBuilder.cpp | 13 ++++ .../SelectionDAG/SelectionDAGBuilder.h | 4 ++ lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 8 ++- .../X86/2010-09-30-CMOV-JumpTable-PHI.ll | 71 +++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/CodeGen/X86/2010-09-30-CMOV-JumpTable-PHI.ll diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index c316ef6c54a..adc225b94bc 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -2207,6 +2207,19 @@ size_t SelectionDAGBuilder::Clusterify(CaseVector& Cases, return numCmps; } +void SelectionDAGBuilder::UpdateSplitBlock(MachineBasicBlock *First, + MachineBasicBlock *Last) { + // Update JTCases. + for (unsigned i = 0, e = JTCases.size(); i != e; ++i) + if (JTCases[i].first.HeaderBB == First) + JTCases[i].first.HeaderBB = Last; + + // Update BitTestCases. + for (unsigned i = 0, e = BitTestCases.size(); i != e; ++i) + if (BitTestCases[i].Parent == First) + BitTestCases[i].Parent = Last; +} + void SelectionDAGBuilder::visitSwitch(const SwitchInst &SI) { MachineBasicBlock *SwitchMBB = FuncInfo.MBB; diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h index 5f400e9c83a..dfd1df75f10 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h @@ -398,6 +398,10 @@ public: void LowerCallTo(ImmutableCallSite CS, SDValue Callee, bool IsTailCall, MachineBasicBlock *LandingPad = NULL); + /// UpdateSplitBlock - When an MBB was split during scheduling, update the + /// references that ned to refer to the last resulting block. + void UpdateSplitBlock(MachineBasicBlock *First, MachineBasicBlock *Last); + private: // Terminator instructions. void visitRet(const ReturnInst &I); diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 63ba1c415ae..450b7eff6c9 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -564,13 +564,19 @@ void SelectionDAGISel::CodeGenAndEmitDAG() { // Emit machine code to BB. This can change 'BB' to the last block being // inserted into. + MachineBasicBlock *FirstMBB = FuncInfo->MBB, *LastMBB; { NamedRegionTimer T("Instruction Creation", GroupName, TimePassesIsEnabled); - FuncInfo->MBB = Scheduler->EmitSchedule(); + LastMBB = FuncInfo->MBB = Scheduler->EmitSchedule(); FuncInfo->InsertPt = Scheduler->InsertPos; } + // If the block was split, make sure we update any references that are used to + // update PHI nodes later on. + if (FirstMBB != LastMBB) + SDB->UpdateSplitBlock(FirstMBB, LastMBB); + // Free the scheduler state. { NamedRegionTimer T("Instruction Scheduling Cleanup", GroupName, diff --git a/test/CodeGen/X86/2010-09-30-CMOV-JumpTable-PHI.ll b/test/CodeGen/X86/2010-09-30-CMOV-JumpTable-PHI.ll new file mode 100644 index 00000000000..cae81d086ea --- /dev/null +++ b/test/CodeGen/X86/2010-09-30-CMOV-JumpTable-PHI.ll @@ -0,0 +1,71 @@ +; RUN: llc -verify-machineinstrs -cgp-critical-edge-splitting=0 -mcpu=i386 < %s +target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:32:32-n8:16:32" +target triple = "i386-pc-linux-gnu" + +; The bb.i basic block gets split while emitting the schedule because +; -mcpu=i386 doesn't have CMOV.' +; +; That causes the PHI to be updated wrong because the jumptable data structure is remembering the original MBB. +; +; -cgp-critical-edge-splitting=0 prevents the edge to PHI from being split. + +@.str146 = external constant [4 x i8], align 1 +@.str706 = external constant [4 x i8], align 1 +@.str1189 = external constant [5 x i8], align 1 + +declare i32 @memcmp(i8* nocapture, i8* nocapture, i32) nounwind readonly +declare i32 @strlen(i8* nocapture) nounwind readonly + +define hidden zeroext i8 @f(i8* %this, i8* %Name.0, i32 %Name.1, i8* noalias %NameLoc, i8* %Operands) nounwind align 2 { +bb.i: + %0 = icmp eq i8 undef, 0 + %iftmp.285.0 = select i1 %0, i8* getelementptr inbounds ([5 x i8]* @.str1189, i32 0, i32 0), i8* getelementptr inbounds ([4 x i8]* @.str706, i32 0, i32 0) + %1 = call i32 @strlen(i8* %iftmp.285.0) nounwind readonly + switch i32 %Name.1, label %_ZNK4llvm12StringSwitchINS_9StringRefES1_E7DefaultERKS1_.exit [ + i32 3, label %bb1.i + i32 4, label %bb1.i1237 + i32 5, label %bb1.i1266 + i32 6, label %bb1.i1275 + i32 2, label %bb1.i1434 + i32 8, label %bb1.i1523 + i32 7, label %bb1.i1537 + ] + +bb1.i: ; preds = %bb.i + unreachable + +bb1.i1237: ; preds = %bb.i + br i1 undef, label %bb.i1820, label %bb1.i1241 + +bb1.i1241: ; preds = %bb1.i1237 + unreachable + +bb1.i1266: ; preds = %bb.i + unreachable + +bb1.i1275: ; preds = %bb.i + unreachable + +bb1.i1434: ; preds = %bb.i + unreachable + +bb1.i1523: ; preds = %bb.i + unreachable + +bb1.i1537: ; preds = %bb.i + unreachable + +bb.i1820: ; preds = %bb1.i1237 + br label %_ZNK4llvm12StringSwitchINS_9StringRefES1_E7DefaultERKS1_.exit + +_ZNK4llvm12StringSwitchINS_9StringRefES1_E7DefaultERKS1_.exit: ; preds = %bb.i1820, %bb.i + %PatchedName.0.0 = phi i8* [ undef, %bb.i1820 ], [ %Name.0, %bb.i ] + br i1 undef, label %bb141, label %_ZNK4llvm9StringRef10startswithES0_.exit + +_ZNK4llvm9StringRef10startswithES0_.exit: ; preds = %_ZNK4llvm12StringSwitchINS_9StringRefES1_E7DefaultERKS1_.exit + %2 = call i32 @memcmp(i8* %PatchedName.0.0, i8* getelementptr inbounds ([4 x i8]* @.str146, i32 0, i32 0), i32 3) nounwind readonly + unreachable + +bb141: ; preds = %_ZNK4llvm12StringSwitchINS_9StringRefES1_E7DefaultERKS1_.exit + unreachable +} -- 2.34.1