ScheduleDAG: record PhysReg dependencies represented by CopyFromReg nodes
authorTim Northover <tnorthover@apple.com>
Thu, 23 Oct 2014 22:31:48 +0000 (22:31 +0000)
committerTim Northover <tnorthover@apple.com>
Thu, 23 Oct 2014 22:31:48 +0000 (22:31 +0000)
x86's CMPXCHG -> EFLAGS consumer wasn't being recorded as a real EFLAGS
dependency because it was represented by a pair of CopyFromReg(EFLAGS) ->
CopyToReg(EFLAGS) nodes. ScheduleDAG was expecting the source to be an
implicit-def on the instruction, where the result numbers in the DAG and the
Uses list in TableGen matched up precisely.

The Copy notation seems much more robust, so this patch extends ScheduleDAG
rather than refactoring x86.

Should fix PR20376.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220529 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp
lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp
lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
test/CodeGen/X86/cmpxchg-clobber-flags.ll [new file with mode: 0644]

index 4d8c2c78bce62d92ffc223ebd9ce48019c05b3f3..412dea55de55ead735b7b542211b13217ffdac3c 100644 (file)
@@ -433,13 +433,19 @@ void ScheduleDAGFast::InsertCopiesAndMoveSuccs(SUnit *SU, unsigned Reg,
 /// FIXME: Move to SelectionDAG?
 static EVT getPhysicalRegisterVT(SDNode *N, unsigned Reg,
                                  const TargetInstrInfo *TII) {
 /// FIXME: Move to SelectionDAG?
 static EVT getPhysicalRegisterVT(SDNode *N, unsigned Reg,
                                  const TargetInstrInfo *TII) {
-  const MCInstrDesc &MCID = TII->get(N->getMachineOpcode());
-  assert(MCID.ImplicitDefs && "Physical reg def must be in implicit def list!");
-  unsigned NumRes = MCID.getNumDefs();
-  for (const uint16_t *ImpDef = MCID.getImplicitDefs(); *ImpDef; ++ImpDef) {
-    if (Reg == *ImpDef)
-      break;
-    ++NumRes;
+  unsigned NumRes;
+  if (N->getOpcode() == ISD::CopyFromReg) {
+    // CopyFromReg has: "chain, Val, glue" so operand 1 gives the type.
+    NumRes = 1;
+  } else {
+    const MCInstrDesc &MCID = TII->get(N->getMachineOpcode());
+    assert(MCID.ImplicitDefs && "Physical reg def must be in implicit def list!");
+    NumRes = MCID.getNumDefs();
+    for (const uint16_t *ImpDef = MCID.getImplicitDefs(); *ImpDef; ++ImpDef) {
+      if (Reg == *ImpDef)
+        break;
+      ++NumRes;
+    }
   }
   return N->getValueType(NumRes);
 }
   }
   return N->getValueType(NumRes);
 }
index d3d842abdd27c95f11cda4842208277f414f22af..993bdb3d6cc796521163dbc418669e2689d8809f 100644 (file)
@@ -1190,13 +1190,19 @@ void ScheduleDAGRRList::InsertCopiesAndMoveSuccs(SUnit *SU, unsigned Reg,
 /// FIXME: Move to SelectionDAG?
 static EVT getPhysicalRegisterVT(SDNode *N, unsigned Reg,
                                  const TargetInstrInfo *TII) {
 /// FIXME: Move to SelectionDAG?
 static EVT getPhysicalRegisterVT(SDNode *N, unsigned Reg,
                                  const TargetInstrInfo *TII) {
-  const MCInstrDesc &MCID = TII->get(N->getMachineOpcode());
-  assert(MCID.ImplicitDefs && "Physical reg def must be in implicit def list!");
-  unsigned NumRes = MCID.getNumDefs();
-  for (const uint16_t *ImpDef = MCID.getImplicitDefs(); *ImpDef; ++ImpDef) {
-    if (Reg == *ImpDef)
-      break;
-    ++NumRes;
+  unsigned NumRes;
+  if (N->getOpcode() == ISD::CopyFromReg) {
+    // CopyFromReg has: "chain, Val, glue" so operand 1 gives the type.
+    NumRes = 1;
+  } else {
+    const MCInstrDesc &MCID = TII->get(N->getMachineOpcode());
+    assert(MCID.ImplicitDefs && "Physical reg def must be in implicit def list!");
+    NumRes = MCID.getNumDefs();
+    for (const uint16_t *ImpDef = MCID.getImplicitDefs(); *ImpDef; ++ImpDef) {
+      if (Reg == *ImpDef)
+        break;
+      ++NumRes;
+    }
   }
   return N->getValueType(NumRes);
 }
   }
   return N->getValueType(NumRes);
 }
index 2005f296feac1c7acb6370cc19a6cf8b6482292e..fd4a35eb5edcc6ce2ab58482436c4800d617fad3 100644 (file)
@@ -119,15 +119,20 @@ static void CheckForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
     return;
 
   unsigned ResNo = User->getOperand(2).getResNo();
     return;
 
   unsigned ResNo = User->getOperand(2).getResNo();
-  if (Def->isMachineOpcode()) {
+  if (Def->getOpcode() == ISD::CopyFromReg &&
+      cast<RegisterSDNode>(Def->getOperand(1))->getReg() == Reg) {
+    PhysReg = Reg;
+  } else if (Def->isMachineOpcode()) {
     const MCInstrDesc &II = TII->get(Def->getMachineOpcode());
     if (ResNo >= II.getNumDefs() &&
     const MCInstrDesc &II = TII->get(Def->getMachineOpcode());
     if (ResNo >= II.getNumDefs() &&
-        II.ImplicitDefs[ResNo - II.getNumDefs()] == Reg) {
+        II.ImplicitDefs[ResNo - II.getNumDefs()] == Reg)
       PhysReg = Reg;
       PhysReg = Reg;
-      const TargetRegisterClass *RC =
+  }
+
+  if (PhysReg != 0) {
+    const TargetRegisterClass *RC =
         TRI->getMinimalPhysRegClass(Reg, Def->getValueType(ResNo));
         TRI->getMinimalPhysRegClass(Reg, Def->getValueType(ResNo));
-      Cost = RC->getCopyCost();
-    }
+    Cost = RC->getCopyCost();
   }
 }
 
   }
 }
 
diff --git a/test/CodeGen/X86/cmpxchg-clobber-flags.ll b/test/CodeGen/X86/cmpxchg-clobber-flags.ll
new file mode 100644 (file)
index 0000000..3cb8b97
--- /dev/null
@@ -0,0 +1,86 @@
+; RUN: llc -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s
+; RUN: llc -mtriple=x86_64-linux-gnu -pre-RA-sched=fast %s -o - | FileCheck %s
+
+declare i32 @bar()
+
+define i64 @test_intervening_call(i64* %foo, i64 %bar, i64 %baz) {
+; CHECK-LABEL: test_intervening_call:
+; CHECK: cmpxchg
+; CHECK: pushfq
+; CHECK: popq [[FLAGS:%.*]]
+
+; CHECK: callq bar
+
+; CHECK: pushq [[FLAGS]]
+; CHECK: popfq
+; CHECK: jne
+  %cx = cmpxchg i64* %foo, i64 %bar, i64 %baz seq_cst seq_cst
+  %p = extractvalue { i64, i1 } %cx, 1
+  call i32 @bar()
+  br i1 %p, label %t, label %f
+
+t:
+  ret i64 42
+
+f:
+  ret i64 0
+}
+
+; Interesting in producing a clobber without any function calls.
+define i32 @test_control_flow(i32* %p, i32 %i, i32 %j) {
+; CHECK-LABEL: test_control_flow:
+
+; CHECK: cmpxchg
+; CHECK-NEXT: jne
+entry:
+  %cmp = icmp sgt i32 %i, %j
+  br i1 %cmp, label %loop_start, label %cond.end
+
+loop_start:
+  br label %while.condthread-pre-split.i
+
+while.condthread-pre-split.i:
+  %.pr.i = load i32* %p, align 4
+  br label %while.cond.i
+
+while.cond.i:
+  %0 = phi i32 [ %.pr.i, %while.condthread-pre-split.i ], [ 0, %while.cond.i ]
+  %tobool.i = icmp eq i32 %0, 0
+  br i1 %tobool.i, label %while.cond.i, label %while.body.i
+
+while.body.i:
+  %.lcssa = phi i32 [ %0, %while.cond.i ]
+  %1 = cmpxchg i32* %p, i32 %.lcssa, i32 %.lcssa seq_cst seq_cst
+  %2 = extractvalue { i32, i1 } %1, 1
+  br i1 %2, label %cond.end.loopexit, label %while.condthread-pre-split.i
+
+cond.end.loopexit:
+  br label %cond.end
+
+cond.end:
+  %cond = phi i32 [ %i, %entry ], [ 0, %cond.end.loopexit ]
+  ret i32 %cond
+}
+
+; This one is an interesting case because CMOV doesn't have a chain
+; operand. Naive attempts to limit cmpxchg EFLAGS use are likely to fail here.
+define i32 @test_feed_cmov(i32* %addr, i32 %desired, i32 %new) {
+; CHECK-LABEL: test_feed_cmov:
+
+; CHECK: cmpxchg
+; CHECK: pushfq
+; CHECK: popq [[FLAGS:%.*]]
+
+; CHECK: callq bar
+
+; CHECK: pushq [[FLAGS]]
+; CHECK: popfq
+
+  %res = cmpxchg i32* %addr, i32 %desired, i32 %new seq_cst seq_cst
+  %success = extractvalue { i32, i1 } %res, 1
+
+  %rhs = call i32 @bar()
+
+  %ret = select i1 %success, i32 %new, i32 %rhs
+  ret i32 %ret
+}