[PATCH] D47569: [Power9]Legalize and emit code for quad-precision convert from single-precision

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:801
         setLoadExtAction(ISD::EXTLOAD, MVT::f128, MVT::f64, Expand);
+        setLoadExtAction(ISD::EXTLOAD, MVT::f128, MVT::f32, Expand);
         setOperationAction(ISD::FMA, MVT::f128, Legal);
----------------
I imagine this just goes away since it's handled in the other patch.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3386
+  def : Pat<(f128 (fpextend f32:$src)),
+            (f128 (XSCVDPQP (XSCPSGNDP (COPY_TO_REGCLASS $src, VSFRC),
+                                       (COPY_TO_REGCLASS $src, VSFRC))))>;
----------------
Huh? We are copying the sign of the input to itself? That seems like an unnecessary noop. Why do we need that?


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3388
+                                       (COPY_TO_REGCLASS $src, VSFRC))))>;
+  def : Pat<(f128 (fpextend (f32 (load ixaddr:$src)))),
+            (f128 (XSCVDPQP (LXSSP ixaddr:$src)))>;
----------------
These two patterns seem:
1. Redundant since we can handle loading and extending separately already anyway
2. Wrong if the weird sign copying sequence is actually necessary

We will produce a different sequence for these two equivalent snippets of code and that seems wrong:
```
__float128 test1(float *Ptr) {
  return *Ptr;
}
```
vs.
```
float __attribute__((noinline)) getFromPtr(float *Ptr) { return *Ptr; }
__float128 test2(float *Ptr) {
  return getFromPtr(Ptr);
}
```


https://reviews.llvm.org/D47569