[PATCH] D69275: Add constrained int->FP intrinsics

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:988
     setOperationAction(ISD::SINT_TO_FP,         MVT::v4i32, Legal);
+    setOperationAction(ISD::STRICT_SINT_TO_FP, MVT::v4i32, Legal);
     setOperationAction(ISD::SINT_TO_FP,         MVT::v2i32, Custom);
----------------
Line the arguments up with the l ine above. Same for the two below


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18521
   if (SrcVT.isVector()) {
-    if (SrcVT == MVT::v2i32 && VT == MVT::v2f64) {
+    if (SrcVT == MVT::v2i32 && VT == MVT::v2f64 &&
+        !Op.getNode()->isStrictFPOpcode()) {
----------------
Please add a FIXME


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18696
+    Sub = DAG.getNode(ISD::STRICT_FSUB, dl, {MVT::v2f64, MVT::Other},
+                      {Op.getOperand(0), XR2F, CLod1});
+    Chain = SDValue(Sub.getNode(), 1);
----------------
getValue


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18697
+                      {Op.getOperand(0), XR2F, CLod1});
+    Chain = SDValue(Sub.getNode(), 1);
+  } else
----------------
getValue


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18717
+                         DAG.getIntPtrConstant(0, dl));
+    return DAG.getNode(ISD::MERGE_VALUES, dl, Op->getVTList(), Result, Chain);
   }
----------------
You can DAG.getMergeValues which takes less arguments


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18764
+    return DAG
+        .getStrictFPExtendOrRound(Sub, SDValue(Sub.getNode(), 1), dl,
+                                  Op.getSimpleValueType())
----------------
Use Sub.getValue(1)


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18766
+                                  Op.getSimpleValueType())
+        .first;
+  }
----------------
Don't we need to propagate the chain result too?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18783
 
+  if (Op.getNode()->isStrictFPOpcode())
+    return SDValue();
----------------
Pleas add a FIXME here


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19011
                                          MVT::i64, MMO);
+  Chain = SDValue(Fild.getNode(), 1);
 
----------------
You can just use Fild.getValue(1)


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19036
       /* Alignment = */ 4);
+  Chain = SDValue(Fudge.getNode(), 1);
   // Extend everything to 80 bits to force it to be done on x87.
----------------
Use getValue


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19044
+        ISD::STRICT_FP_ROUND, dl, {DstVT, MVT::Other},
+        {SDValue(Add.getNode(), 1), Add, DAG.getIntPtrConstant(0, dl)});
+  }
----------------
Use getValue


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69275/new/

https://reviews.llvm.org/D69275