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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai marked 4 inline comments as done.
nemanjai added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:184
+    setTruncStoreAction(MVT::f32, MVT::f16, Expand);
+  }
+
----------------
shchenz wrote:
> Do we need to handle ppcf128 also? 
Not really. That type is really just a pair of doubles and there is no register that can contain it, so it will always be broken up into a pair of doubles by the legalizer.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:114
                         [SDNPHasChain, SDNPMayLoad, SDNPMemOperand]>;
+def extloadf16 : PatFrag<(ops node:$ptr), (extload node:$ptr)> {
+  let IsLoad = 1;
----------------
shchenz wrote:
> Guard under IsISA3_0?
Why? This is just a def of a pattern fragment. It is only defined here because it is missing in the target independent td file.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3263
             (v2i64 (XXPERMDIs (VEXTSH2Ds (LXSIHZX xoaddr:$src)), 0))>;
 
+  // Load/convert and convert/store patterns for f16.
----------------
shchenz wrote:
> Guard under IsISA3_0?
The instructions used are defined in a `Power9Vector` guard and so are these patterns. I realize this is hard to track down from the patch - this file is in desperate need of refactoring :(


================
Comment at: test/CodeGen/PowerPC/handle-f16-storage-type.ll:8
+; Function Attrs: nounwind readonly
+define dso_local double @loadd(i16* nocapture readonly %a) local_unnamed_addr #0 {
+; P8-LABEL: loadd:
----------------
shchenz wrote:
> `#0 `, seems all the function attributes are not defined?
Sure, I can get rid of these (actually, I'll just define #0 as `nounwind` and use it for all the functions so we don't get the CFI nodes).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68237