[PATCH] [RFC] Lame fix for PR20376

[Peter Collingbourne]

The problem (as I understand it) is the DAG eventually ends up representing
a necessary EFLAGS copy. But we deliberately do not perform EFLAGS copies
because they are impossible on x86 (see InstrEmitter.cpp:171 or thereabouts),
so we end up using whatever happens to be in EFLAGS at the time of the branch.
This problem does not end up affecting compares/overflow checks because
they are schedule-independent, so we can easily move (or copy) the
instruction to wherever is convenient.

The workaround I came up with was to disable a simplification that may lead
to such a copy occurring, specifically the one that eliminates SETCC nodes, if
they refer to a CopyFromReg which may not be scheduled right before the branch,
in the hope that the SETCC will be scheduled in the right place. This turned
out to be not so great; a number of tests now fail because of extra copies.

One other idea I had was a peephole MI pass that removes SETCC instructions
if safe to do so. Does that sound like a good idea?

There may be a much better way of solving this. I'm not a backend expert;
this is what I have gleaned after staring at selection DAGs for far too
long.

http://reviews.llvm.org/D4611

Files:
  lib/Target/X86/X86ISelLowering.cpp

Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp
+++ lib/Target/X86/X86ISelLowering.cpp
@@ -19820,7 +19820,8 @@
 //
 // where Op could be BRCOND or CMOV.
 //
-static SDValue checkBoolTestSetCCCombine(SDValue Cmp, X86::CondCode &CC) {
+static SDValue checkBoolTestSetCCCombine(SDValue Chain, SDValue Cmp,
+                                         X86::CondCode &CC) {
   // Quit if not CMP and SUB with its value result used.
   if (Cmp.getOpcode() != X86ISD::CMP &&
       (Cmp.getOpcode() != X86ISD::SUB || Cmp.getNode()->hasAnyUseOfValue(0)))
@@ -19887,12 +19888,21 @@
     assert(X86::CondCode(SetCC.getConstantOperandVal(0)) == X86::COND_B &&
            "Invalid use of SETCC_CARRY!");
     // FALL THROUGH
-  case X86ISD::SETCC:
+  case X86ISD::SETCC: {
+    // If the operand is CopyFromReg, applying this simplification may create
+    // a situation later where we need to copy EFLAGS. Since this isn't
+    // supported, we will generate incorrect code. To avoid this situation,
+    // we forbid this simplification if the operand is CopyFromReg and it
+    // is not directly chained to us.
+    SDValue Op = SetCC.getOperand(1);
+    if (Chain && Op.getOpcode() == ISD::CopyFromReg && Op.getValue(1) != Chain)
+      break;
     // Set the condition code or opposite one if necessary.
     CC = X86::CondCode(SetCC.getConstantOperandVal(0));
     if (needOppositeCond)
       CC = X86::GetOppositeBranchCondition(CC);
-    return SetCC.getOperand(1);
+    return Op;
+  }
   case X86ISD::CMOV: {
     // Check whether false/true value has canonical one, i.e. 0 or 1.
     ConstantSDNode *FVal = dyn_cast<ConstantSDNode>(SetCC.getOperand(0));
@@ -19965,7 +19975,7 @@
 
   SDValue Flags;
 
-  Flags = checkBoolTestSetCCCombine(Cond, CC);
+  Flags = checkBoolTestSetCCCombine(SDValue(), Cond, CC);
   if (Flags.getNode() &&
       // Extra check as FCMOV only supports a subset of X86 cond.
       (FalseOp.getValueType() != MVT::f80 || hasFPCMov(CC))) {
@@ -21803,7 +21813,7 @@
 
   SDValue Flags;
 
-  Flags = checkBoolTestSetCCCombine(EFLAGS, CC);
+  Flags = checkBoolTestSetCCCombine(SDValue(), EFLAGS, CC);
   if (Flags.getNode()) {
     SDValue Cond = DAG.getConstant(CC, MVT::i8);
     return DAG.getNode(X86ISD::SETCC, DL, N->getVTList(), Cond, Flags);
@@ -21825,7 +21835,7 @@
 
   SDValue Flags;
 
-  Flags = checkBoolTestSetCCCombine(EFLAGS, CC);
+  Flags = checkBoolTestSetCCCombine(Chain, EFLAGS, CC);
   if (Flags.getNode()) {
     SDValue Cond = DAG.getConstant(CC, MVT::i8);
     return DAG.getNode(X86ISD::BRCOND, DL, N->getVTList(), Chain, Dest, Cond,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4611.11733.patch
Type: text/x-patch
Size: 2679 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140722/13c404d9/attachment.bin>