[PATCH] D46997: [Power9]Legalize and emit code for round & convert quad-precision values

[Lei Huang via Phabricator via llvm-commits]

lei added inline comments.


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:442
+                        (outs vssrc:$XT), (ins vsfrc:$XB),
+                        "xsrsp $XT, $XB", IIC_VecFP, []>;
   def XSRSQRTEDP : XX2Form<60, 74,
----------------
nemanjai wrote:
> This should probably have a pattern (the same pattern as the definition of `FRSP`) so that we can do these conversions in all 64 registers without having to move stuff around.
> 
> Of course, doing that will probably change behaviour of a number of test cases that will have to be updated. It will probably require a change in `PPCMIPeephole.cpp` since we explicitly check for `PPC::FRSP`.
> But it does need to be done. We clearly missed this instruction when we implemented support for ISA 2.07.
> 
> Finally, I think this is in the wrong place. This was introduced in ISA 2.07 (Power8) so it needs to be in a `HasP8Vector` block.
I will move this definition to down to the ISA 2.07 instruction implementation block. However I will leave out the pattern for implementation in a different, non float128, patch.  


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:2374
+  // [PO VRT XO VRB XO /]
+  class X_VT5_XO5_VB5_VFSR<bits<6> opcode, bits<5> xo2, bits<10> xo, string opc,
+                      list<dag> pattern>
----------------
nemanjai wrote:
> What does the acronym `VFSR` stand for? Is it meant to be `VSFR` (as in vector-scalar FP register)?
ops.. yes it was meant to be `VSFR`


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:3378
+  def : Pat<(f64 (fpround f128:$src)), (f64 (XSCVQPDP $src))>;
+  def : Pat<(store (f64 (fpround f128:$src)), xaddr:$dst),
+            (STXSDX (XSCVQPDP f128:$src), xaddr:$dst)>;
----------------
nemanjai wrote:
> The storage patterns seem redundant. Won't we emit the exact same code even without them?
The output register for the `X_VT5_XO5_VB5_VFSR` class was wrong.  Will update to `vfrc` from `vsfrc` and remove these storage patterns.


https://reviews.llvm.org/D46997