[PATCH] D68237: [PowerPC] Handle f16 as a storage type only

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:170
 
+  if (Subtarget.isISA3_0()) {
+    setLoadExtAction(ISD::EXTLOAD, MVT::f64, MVT::f16, Legal);
----------------
Do we need to consider `useSoftFloat` here?


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:114
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
+def extloadf16 : PatFrag<(ops node:$ptr), (extload node:$ptr)> {
+  let IsLoad = 1;
----------------
Why we need such PatFrag while the other targets with fp16 support not?  Is it due to that we only want to handle storage types in this patch?

If this is really needed, why not move it to `TargetSelectionDAG.td` to be together with `extloadf32` and `extloadf64`?


================
Comment at: llvm/test/CodeGen/PowerPC/handle-f16-storage-type.ll:35
+
+; Function Attrs: nounwind readnone willreturn
+declare double @llvm.convert.from.fp16.f64(i16) #1
----------------
Nit: Function Attrs deleted but comments still kept.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68237