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.

llvm-svn: 220529

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

index 4d8c2c7..412dea5 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) {
-  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);
 }
index d3d842a..993bdb3 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) {
-  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);
 }
index 2005f29..fd4a35e 100644 (file)
@@ -119,15 +119,20 @@ static void CheckForPhysRegDependency(SDNode *Def, SDNode *User, unsigned Op,
     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() &&
-        II.ImplicitDefs[ResNo - II.getNumDefs()] == Reg) {
+        II.ImplicitDefs[ResNo - II.getNumDefs()] == Reg)
       PhysReg = Reg;
-      const TargetRegisterClass *RC =
+  }
+
+  if (PhysReg != 0) {
+    const TargetRegisterClass *RC =
         TRI->getMinimalPhysRegClass(Reg, Def->getValueType(ResNo));
-      Cost = RC->getCopyCost();
-    }
+    Cost = RC->getCopyCost();
   }
 }
 
diff --git a/llvm/test/CodeGen/X86/cmpxchg-clobber-flags.ll b/llvm/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
+}