[PATCH] D109421: [PowerPC] Add intrinsic to convert between ppc_fp128 and fp128

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

Thanks for doing this. Looks reasonable to me. Some code format comments.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10390
+    std::pair<SDValue, SDValue> Result =
+      makeLibCall(DAG, LC, MVT::f128, Op.getOperand(1),
+                  CallOptions, dl, SDValue());
----------------
Lint warnings.

Do we need a local variable for `LC`?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11031
   }
+  case ISD::INTRINSIC_WO_CHAIN: {
+    if (cast<ConstantSDNode>(N->getOperand(0))->getZExtValue() !=
----------------
Can we put this also in `LowerINTRINSIC_WO_CHAIN`? and call `LowerINTRINSIC_WO_CHAIN` here? So we can have a concentrated place for the ppc128/f128 conversion?


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:11039
+    std::pair<SDValue, SDValue> Tmp =
+        makeLibCall(DAG, RTLIB::CONVERT_F128_PPCF128, MVT::ppcf128, Op,
+                    CallOptions, dl, SDValue());
----------------
no need for local `Op`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109421