From c0ecebb32b4c97319b5974f60be56c4a5f186684 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Thu, 23 Oct 2014 22:31:48 +0000 Subject: [PATCH] ScheduleDAG: record PhysReg dependencies represented by CopyFromReg nodes 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 | 20 +++-- .../SelectionDAG/ScheduleDAGRRList.cpp | 20 +++-- .../SelectionDAG/ScheduleDAGSDNodes.cpp | 15 ++-- test/CodeGen/X86/cmpxchg-clobber-flags.ll | 86 +++++++++++++++++++ 4 files changed, 122 insertions(+), 19 deletions(-) create mode 100644 test/CodeGen/X86/cmpxchg-clobber-flags.ll diff --git a/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp b/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp index 4d8c2c78bce..412dea55de5 100644 --- a/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp +++ b/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp @@ -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); } diff --git a/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp b/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp index d3d842abdd2..993bdb3d6cc 100644 --- a/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp +++ b/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp @@ -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); } diff --git a/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp index 2005f296fea..fd4a35eb5ed 100644 --- a/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp +++ b/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp @@ -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(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/test/CodeGen/X86/cmpxchg-clobber-flags.ll b/test/CodeGen/X86/cmpxchg-clobber-flags.ll new file mode 100644 index 00000000000..3cb8b97de45 --- /dev/null +++ b/test/CodeGen/X86/cmpxchg-clobber-flags.ll @@ -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 +} -- 2.34.1