[llvm-commits] [PATCH] fix PR13577, an issue caused by r161687

[Chandler Carruth]

On Sat, Aug 11, 2012 at 3:49 PM, Michael Liao <michael.liao at intel.com>wrote:

> Hi
>
> Sorry, PR13577 and recent failures of compiler-rt failures is introduced
> by r161687. The root cause is that FCMOV (only used for 80-bit double)
> only supports a subset of X86 conditions. The attached patch fixes that
> by checking whether it's a CMOV on f80 and whether the condition is
> valid. A minimal test case (copysign) for PR13577 is also added.
>

Hmm, my diagnosis of that PR may have been a bit off -- you should check
whether this patch actually fixes it, as it seems to potentially predate
r161687. Fixing it may however re-use this logic.

Either way, that patch seems fine, and should at least fix the failures
David sent you mail about. One nit-pick:

@@ -13877,10 +13893,14 @@ static SDValue PerformCMOVCombine(SDNode *N,
SelectionDAG &DAG,

   Flags = BoolTestSetCCCombine(Cond, CC);
   if (Flags.getNode()) {
-    SDValue Ops[] = { FalseOp, TrueOp,
-                      DAG.getConstant(CC, MVT::i8), Flags };
-    return DAG.getNode(X86ISD::CMOV, DL, N->getVTList(),
-                       Ops, array_lengthof(Ops));
+    // Check for FCMOV which only supports a subset of X86 cond.
+    if (FalseOp.getValueType() != MVT::f80 ||
+        IsValidFCMOVCondition(CC)) {

Just combine this and the above condition:

if (Flags.genNode() && (FalseOp.getValueType() != MVT::f80 || ...)) {
  ...
}

+      SDValue Ops[] = { FalseOp, TrueOp,
+                        DAG.getConstant(CC, MVT::i8), Flags };
+      return DAG.getNode(X86ISD::CMOV, DL, N->getVTList(),
+                         Ops, array_lengthof(Ops));
+    }
   }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120811/e80432f1/attachment.html>