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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2397
+    SDValue Sub;
+    if (Node->isStrictFPOpcode()) {
+      Sub = DAG.getNode(ISD::STRICT_FSUB, dl, {MVT::f64, MVT::Other},
----------------
Can we merge the 2 strict fp ifs here? The only thing we do between them is declare a new variable.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2405
+    if (Node->isStrictFPOpcode()) {
+      if (!DestVT.bitsEq(Sub.getValueType())) {
+        std::pair<SDValue, SDValue> ResultPair;
----------------
Why is this not just DestVT != Sub.getValueType()


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18704
   if (Subtarget.hasSSE3() && shouldUseHorizontalOp(true, DAG, Subtarget)) {
+    // FIXME: Do we need a STRICT version of FHADD?
     Result = DAG.getNode(X86ISD::FHADD, dl, MVT::v2f64, Sub, Sub);
----------------
We probably do need a strict version of FHADD, but until we have that we should just go to the shuffle + STRICT_FADD code below rather than silently dropping the chain.


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

https://reviews.llvm.org/D69275