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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

Do we need a Verifier check to ensure that the vector element counts match between source and dest type? Not sure what we do for the other direction.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2355
                                                    const SDLoc &dl) {
+  SDValue Op0;
+  if (Node->isStrictFPOpcode())
----------------
Ternary on the operand number?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2488
+  }
+  else
+    return DAG.getNode(ISD::FADD, dl, DestVT, Tmp1, FudgeInReg);
----------------
Drop the else since we early returned?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2945
+      return true;
+    } else
+      Tmp1 = ExpandLegalINT_TO_FP(Node->getOpcode() == ISD::SINT_TO_FP,
----------------
Drop the else?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:1110
+             ? getNode(ISD::STRICT_FP_EXTEND, DL, { VT, MVT::Other }, { Chain, Op })
+             : getNode(ISD::STRICT_FP_ROUND, DL, { VT, MVT::Other }, { Chain, Op, getIntPtrConstant(0, DL) });
+}
----------------
Is this longer than 80 columns?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6035
+  SDValue Src;
+  if (Node->isStrictFPOpcode())
+    Src = Node->getOperand(1);
----------------
A ternary operator on the operand number would probably be shorter?


================
Comment at: llvm/test/CodeGen/X86/fp-intrinsics.ll:423
+; CHECK-LABEL: @f31
+; CHECK: call double @llvm.experimental.constrained.sitofp
+define double @f31() #0 {
----------------
This check line is for IR, but this is an assembly test. CHECK isn't a valid check-prefix for this file. Which also means all of the CHECK-LABEL lines are broken in the existing tests aren't doing anything


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69275