ISelDAG: spot chain cycles involving MachineNodes
authorTim Northover <tnorthover@apple.com>
Sun, 22 Sep 2013 08:21:56 +0000 (08:21 +0000)
committerTim Northover <tnorthover@apple.com>
Sun, 22 Sep 2013 08:21:56 +0000 (08:21 +0000)
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

14 files changed:
lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
lib/Target/ARM/ARMISelDAGToDAG.cpp
lib/Target/Hexagon/HexagonISelDAGToDAG.cpp
lib/Target/MSP430/MSP430ISelDAGToDAG.cpp
lib/Target/Mips/MipsISelDAGToDAG.cpp
lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp
lib/Target/PowerPC/PPCISelDAGToDAG.cpp
lib/Target/R600/AMDGPUISelDAGToDAG.cpp
lib/Target/Sparc/SparcISelDAGToDAG.cpp
lib/Target/SystemZ/SystemZISelDAGToDAG.cpp
lib/Target/X86/X86ISelDAGToDAG.cpp
test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll
test/CodeGen/X86/i486-fence-loop.ll [new file with mode: 0644]

index 6066f50c74ebdf725f24242baf4c86ec263ae8c7..9b15c88156f0d0b3bd6cc7eb90ef60881f3c590a 100644 (file)
@@ -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 ||
index ee819e088d89bc0f93043c0c6258f8ae57f4f15b..a8655649f192bb2cf1ec6bcbaf7bff95eb14b241 100644 (file)
@@ -396,6 +396,7 @@ SDNode *AArch64DAGToDAGISel::Select(SDNode *Node) {
 
   if (Node->isMachineOpcode()) {
     DEBUG(dbgs() << "== "; Node->dump(CurDAG); dbgs() << "\n");
+    Node->setNodeId(-1);
     return NULL;
   }
 
index c2dbcb9750ced4bb70dc2d930789aa2b36a38709..f6b38279b32012001bc4f513b6e9d0c6a612f0d4 100644 (file)
@@ -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;
index 9e78e518c2fe4201678bf6a89a0462d43c17306c..5ae93284269b4cb3e256a63b2544d6f13b6d25eb 100644 (file)
@@ -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()) {
index 543f54c9c1be1b532c1813ccd2ac6b76f4af3048..4152829b60daf6f88b3664cb02aac14540fb5d80 100644 (file)
@@ -395,6 +395,7 @@ SDNode *MSP430DAGToDAGISel::Select(SDNode *Node) {
     DEBUG(errs() << "== ";
           Node->dump(CurDAG);
           errs() << "\n");
+    Node->setNodeId(-1);
     return NULL;
   }
 
index 725d9b4686406e8a5b4ac383c400450101c20ffa..b428589c4e41c797fbffce99a27035c53be5e0f7 100644 (file)
@@ -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;
   }
 
index 01fbdb3336720ca53478a015d80b74ea30ad1540..68fa95519b3774bfb5edd1b3fd18f45247536d1c 100644 (file)
@@ -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()) {
index 475bde18efb7ac92445cf45f51fb89e276b72224..6ba6af6446e5c14163a98e64d52edbf6f51fad01 100644 (file)
@@ -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;
index a008c9618d594196c3e55d5fc449e5e4ed2a25b6..88b375b02187cfcb1cf73308d066af7c879da489 100644 (file)
@@ -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) {
index e17a187241c0b1c675c3780cedf0ed2a82b24d35..b012bfdb01025fde4980ff92d3a691c972ac72d2 100644 (file)
@@ -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;
index 01f008a2d98abc5355fd680f612320e147229ba0..d7174087cd57276506d320c08ec9a56c1cc90582 100644 (file)
@@ -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;
   }
 
index 32ad1aaa60461b1657aac3e1a42ee4b3deeb007a..36d16907bfef45b64d14575b569e856842b9bbe8 100644 (file)
@@ -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.
   }
 
index a883d7938b55cd73a2cfd3094837c987da4f2db7..cd8a16f5732a2304c7d1d04417d8e17a16b6fe59 100644 (file)
@@ -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 (file)
index 0000000..d809619
--- /dev/null
@@ -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