[PATCH] D91527: [PowerPC][FP128] Fix the incorrect calling convention for IEEE long double on Power8

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:124
 
+// TODO - Remove this opton if fp128 has been fully supported on Power8 
+static cl::opt<bool>
----------------
qiucf wrote:
> `opton` -> `option`
Good catch.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:126
+static cl::opt<bool>
+    EnableP8FP128("enable-p8-fp128",
+                    cl::desc("temp option to enable fp128 on power8"), cl::Hidden);
----------------
qiucf wrote:
> will it be better if using `enable-soft-fp128` instead of specifying the target is `P8`?
Nice rename !


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:1169
       setOperationAction(ISD::BSWAP, MVT::v1i128, Legal);
+    } else if (Subtarget.hasAltivec() && EnableP8FP128) {
+      addRegisterClass(MVT::f128, &PPC::VRRCRegClass);
----------------
qiucf wrote:
> can we common some code in previous `if` and this `else`? I see some ops are also marked `Expand` under `P9`.
I will common the code when removing the EnableP8FP128 options. So that, it is easier to do the code review. It is ok ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91527