[PATCH] D16110: [Power9] Implement new vsx instructions: quad-precision move, fp-arithmetic

[Nemanja Ivanovic via llvm-commits]

nemanjai added inline comments.

================
Comment at: lib/Target/PowerPC/PPCInstrFormats.td:683
@@ +682,3 @@
+// [PO VRT XO VRB XO RC]
+class XForm_44<bits<6> opcode, bits<5> xo2, bits<10> xo, dag OOL, dag IOL,
+               string asmstr, InstrItinClass itin, list<dag> pattern>
----------------
Last year, we agreed on a naming convention for instruction forms. Please see lib/Target/PowerPC/README.txt (line 626). We have not gotten around to renaming the old ones, but we can at least stick to this naming convention for new forms.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1224
@@ +1223,3 @@
+  def XSCPSGNQP : XForm_18<63, 100,
+                      (outs vrrc:$vT), (ins vrrc:$vA, vrrc:$vB),
+                      "xscpsgnqp $vT, $vA, $vB", IIC_VecFP,
----------------
These are VSX instructions, are they not? Why the VMX register class?
I imagine this is because we have never supported f128 in the VSX registers. However, I don't believe the right course of action is to put these in the VMX registers and thereby be able to allocate only half the registers.
We need to do a bit more to implement support for f128.
I imagine that we can add the type to the VSRC class, but I haven't thought about it much.

Of course, we can simply put in FIXME's to deal with the types later.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1230
@@ +1229,3 @@
+  // TODO: Add intrinsic support, like "class VX1_Int_Ty" in PPCInstrAltivec.td
+  class X44<bits<6> opcode, bits<5> xo2, bits<10> xo, string opc>
+    : XForm_44<opcode, xo2, xo, (outs vrrc:$vT), (ins vrrc:$vB),
----------------
Can we use a more descriptive name than X44? That does not seem to suggest anything about its use.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1231
@@ +1230,3 @@
+  class X44<bits<6> opcode, bits<5> xo2, bits<10> xo, string opc>
+    : XForm_44<opcode, xo2, xo, (outs vrrc:$vT), (ins vrrc:$vB),
+               !strconcat(opc, " $vT, $vB"), IIC_VecFP, []>;
----------------
Same as above.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1244
@@ +1243,3 @@
+  // TODO: Add intrinsic support, or map to llvm instruction
+  class X1<bits<6> opcode, bits<10> xo, string opc>
+    : XForm_1<opcode, xo, (outs vrrc:$vT), (ins vrrc:$vA, vrrc:$vB),
----------------
Same comment about the name as above.


http://reviews.llvm.org/D16110