From 3e84ad28d4d3ceee25771b1e30315c20b7608c39 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Sun, 22 Sep 2013 08:21:56 +0000 Subject: [PATCH] ISelDAG: spot chain cycles involving MachineNodes Previously, the DAGISel function WalkChainUsers was spotting that it had entered already-selected territory by whether a node was a MachineNode (amongst other things). Since it's fairly common practice to insert MachineNodes during ISelLowering, this was not the correct check. Looking around, it seems that other nodes get their NodeId set to -1 upon selection, so this makes sure the same thing happens to all MachineNodes and uses that characteristic to determine whether we should stop looking for a loop during selection. This should fix PR15840. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@191165 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 10 +++---- lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | 1 + lib/Target/ARM/ARMISelDAGToDAG.cpp | 4 ++- lib/Target/Hexagon/HexagonISelDAGToDAG.cpp | 4 ++- lib/Target/MSP430/MSP430ISelDAGToDAG.cpp | 1 + lib/Target/Mips/MipsISelDAGToDAG.cpp | 1 + lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp | 4 ++- lib/Target/PowerPC/PPCISelDAGToDAG.cpp | 4 ++- lib/Target/R600/AMDGPUISelDAGToDAG.cpp | 1 + lib/Target/Sparc/SparcISelDAGToDAG.cpp | 4 ++- lib/Target/SystemZ/SystemZISelDAGToDAG.cpp | 1 + lib/Target/X86/X86ISelDAGToDAG.cpp | 1 + .../X86/2012-01-16-mfence-nosse-flags.ll | 2 +- test/CodeGen/X86/i486-fence-loop.ll | 27 +++++++++++++++++++ 14 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 test/CodeGen/X86/i486-fence-loop.ll diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index 6066f50c74e..9b15c88156f 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -1857,15 +1857,15 @@ WalkChainUsers(const SDNode *ChainedNode, SDNode *User = *UI; + if (User->getOpcode() == ISD::HANDLENODE) // Root of the graph. + continue; + // If we see an already-selected machine node, then we've gone beyond the // pattern that we're selecting down into the already selected chunk of the // DAG. - if (User->isMachineOpcode() || - User->getOpcode() == ISD::HANDLENODE) // Root of the graph. - continue; - unsigned UserOpcode = User->getOpcode(); - if (UserOpcode == ISD::CopyToReg || + if (User->isMachineOpcode() || + UserOpcode == ISD::CopyToReg || UserOpcode == ISD::CopyFromReg || UserOpcode == ISD::INLINEASM || UserOpcode == ISD::EH_LABEL || diff --git a/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp b/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp index ee819e088d8..a8655649f19 100644 --- a/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp +++ b/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp @@ -396,6 +396,7 @@ SDNode *AArch64DAGToDAGISel::Select(SDNode *Node) { if (Node->isMachineOpcode()) { DEBUG(dbgs() << "== "; Node->dump(CurDAG); dbgs() << "\n"); + Node->setNodeId(-1); return NULL; } diff --git a/lib/Target/ARM/ARMISelDAGToDAG.cpp b/lib/Target/ARM/ARMISelDAGToDAG.cpp index c2dbcb9750c..f6b38279b32 100644 --- a/lib/Target/ARM/ARMISelDAGToDAG.cpp +++ b/lib/Target/ARM/ARMISelDAGToDAG.cpp @@ -2383,8 +2383,10 @@ SDNode *ARMDAGToDAGISel::SelectAtomic64(SDNode *Node, unsigned Opc) { SDNode *ARMDAGToDAGISel::Select(SDNode *N) { SDLoc dl(N); - if (N->isMachineOpcode()) + if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. + } switch (N->getOpcode()) { default: break; diff --git a/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp b/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp index 9e78e518c2f..5ae93284269 100644 --- a/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp +++ b/lib/Target/Hexagon/HexagonISelDAGToDAG.cpp @@ -1344,8 +1344,10 @@ SDNode *HexagonDAGToDAGISel::SelectAdd(SDNode *N) { SDNode *HexagonDAGToDAGISel::Select(SDNode *N) { - if (N->isMachineOpcode()) + if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. + } switch (N->getOpcode()) { diff --git a/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp b/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp index 543f54c9c1b..4152829b60d 100644 --- a/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp +++ b/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp @@ -395,6 +395,7 @@ SDNode *MSP430DAGToDAGISel::Select(SDNode *Node) { DEBUG(errs() << "== "; Node->dump(CurDAG); errs() << "\n"); + Node->setNodeId(-1); return NULL; } diff --git a/lib/Target/Mips/MipsISelDAGToDAG.cpp b/lib/Target/Mips/MipsISelDAGToDAG.cpp index 725d9b46864..b428589c4e4 100644 --- a/lib/Target/Mips/MipsISelDAGToDAG.cpp +++ b/lib/Target/Mips/MipsISelDAGToDAG.cpp @@ -110,6 +110,7 @@ SDNode* MipsDAGToDAGISel::Select(SDNode *Node) { // If we have a custom node, we already have selected! if (Node->isMachineOpcode()) { DEBUG(errs() << "== "; Node->dump(CurDAG); errs() << "\n"); + Node->setNodeId(-1); return NULL; } diff --git a/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp b/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp index 01fbdb33367..68fa95519b3 100644 --- a/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp +++ b/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp @@ -118,8 +118,10 @@ bool NVPTXDAGToDAGISel::useF32FTZ() const { /// expanded, promoted and normal instructions. SDNode *NVPTXDAGToDAGISel::Select(SDNode *N) { - if (N->isMachineOpcode()) + if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. + } SDNode *ResNode = NULL; switch (N->getOpcode()) { diff --git a/lib/Target/PowerPC/PPCISelDAGToDAG.cpp b/lib/Target/PowerPC/PPCISelDAGToDAG.cpp index 475bde18efb..6ba6af6446e 100644 --- a/lib/Target/PowerPC/PPCISelDAGToDAG.cpp +++ b/lib/Target/PowerPC/PPCISelDAGToDAG.cpp @@ -876,8 +876,10 @@ SDNode *PPCDAGToDAGISel::SelectSETCC(SDNode *N) { // target-specific node if it hasn't already been changed. SDNode *PPCDAGToDAGISel::Select(SDNode *N) { SDLoc dl(N); - if (N->isMachineOpcode()) + if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. + } switch (N->getOpcode()) { default: break; diff --git a/lib/Target/R600/AMDGPUISelDAGToDAG.cpp b/lib/Target/R600/AMDGPUISelDAGToDAG.cpp index a008c9618d5..88b375b0218 100644 --- a/lib/Target/R600/AMDGPUISelDAGToDAG.cpp +++ b/lib/Target/R600/AMDGPUISelDAGToDAG.cpp @@ -195,6 +195,7 @@ bool AMDGPUDAGToDAGISel::SelectADDR64(SDValue Addr, SDValue& R1, SDValue& R2) { SDNode *AMDGPUDAGToDAGISel::Select(SDNode *N) { unsigned int Opc = N->getOpcode(); if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. } switch (Opc) { diff --git a/lib/Target/Sparc/SparcISelDAGToDAG.cpp b/lib/Target/Sparc/SparcISelDAGToDAG.cpp index e17a187241c..b012bfdb010 100644 --- a/lib/Target/Sparc/SparcISelDAGToDAG.cpp +++ b/lib/Target/Sparc/SparcISelDAGToDAG.cpp @@ -141,8 +141,10 @@ bool SparcDAGToDAGISel::SelectADDRrr(SDValue Addr, SDValue &R1, SDValue &R2) { SDNode *SparcDAGToDAGISel::Select(SDNode *N) { SDLoc dl(N); - if (N->isMachineOpcode()) + if (N->isMachineOpcode()) { + N->setNodeId(-1); return NULL; // Already selected. + } switch (N->getOpcode()) { default: break; diff --git a/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp b/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp index 01f008a2d98..d7174087cd5 100644 --- a/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp +++ b/lib/Target/SystemZ/SystemZISelDAGToDAG.cpp @@ -999,6 +999,7 @@ SDNode *SystemZDAGToDAGISel::Select(SDNode *Node) { // If we have a custom node, we already have selected! if (Node->isMachineOpcode()) { DEBUG(errs() << "== "; Node->dump(CurDAG); errs() << "\n"); + Node->setNodeId(-1); return 0; } diff --git a/lib/Target/X86/X86ISelDAGToDAG.cpp b/lib/Target/X86/X86ISelDAGToDAG.cpp index 32ad1aaa604..36d16907bfe 100644 --- a/lib/Target/X86/X86ISelDAGToDAG.cpp +++ b/lib/Target/X86/X86ISelDAGToDAG.cpp @@ -2057,6 +2057,7 @@ SDNode *X86DAGToDAGISel::Select(SDNode *Node) { if (Node->isMachineOpcode()) { DEBUG(dbgs() << "== "; Node->dump(CurDAG); dbgs() << '\n'); + Node->setNodeId(-1); return NULL; // Already selected. } diff --git a/test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll b/test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll index a883d7938b5..cd8a16f5732 100644 --- a/test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll +++ b/test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll @@ -15,7 +15,7 @@ entry: ; CHECK: lock ; CHECK-NEXT: orl {{.*}}, (%esp) -; CHECK-NEXT: cmpl $0 +; CHECK-NEXT: testl [[REG:%e[a-z]+]], [[REG]] if.then: ; preds = %entry tail call void bitcast (void (...)* @foo to void ()*)() nounwind diff --git a/test/CodeGen/X86/i486-fence-loop.ll b/test/CodeGen/X86/i486-fence-loop.ll new file mode 100644 index 00000000000..d8096197b0d --- /dev/null +++ b/test/CodeGen/X86/i486-fence-loop.ll @@ -0,0 +1,27 @@ +; RUN: llc -march=x86 -mcpu=i486 -o - %s | FileCheck %s + +; Main test here was that ISelDAG could cope with a MachineNode in the chain +; from the first load to the "X86ISD::SUB". Previously it thought that meant no +; cycle could be formed so it tried to use "sub (%eax), [[RHS]]". + +define void @gst_atomic_queue_push(i32* %addr) { +; CHECK-LABEL: gst_atomic_queue_push: +; CHECK: movl (%eax), [[LHS:%e[a-z]+]] +; CHECK: lock +; CHECK-NEXT: orl +; CHECK: movl (%eax), [[RHS:%e[a-z]+]] +; CHECK: cmpl [[LHS]], [[RHS]] + +entry: + br label %while.body + +while.body: + %0 = load volatile i32* %addr, align 4 + fence seq_cst + %1 = load volatile i32* %addr, align 4 + %cmp = icmp sgt i32 %1, %0 + br i1 %cmp, label %while.body, label %if.then + +if.then: + ret void +} \ No newline at end of file