[PATCH] D11471: Scalar to vector conversions using direct moves

Bill Schmidt wschmidt at linux.vnet.ibm.com
Wed Jul 29 11:39:32 PDT 2015


wschmidt added a comment.

A few issues and questions...


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1195
@@ +1194,3 @@
+
+  // Conversions between vector and scalar single precision
+  def XSCVDPSPN : XX2Form<60, 267, (outs vsrc:$XT), (ins vssrc:$XB),
----------------
The comment at line 1195 is wrong.  These are scalar conversions between single and double precision, right?

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1240
@@ +1239,3 @@
+                    (INSERT_SUBREG (i64 (IMPLICIT_DEF)), $A, sub_32), 32, 31));
+  dag DBLWORD_0 = (MTVSRD $A);
+
----------------
BE_DWORD_0?  This isn't an LE pattern, so it needs a BE designator, correct?  Even though it's used in an LE pattern, it is LE doubleword 1 there, so naming it BE_DWORD_0 is more expressive.

================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1247
@@ +1246,3 @@
+  dag LE_DBL = (XXPERMDI LE_CPD, LE_CPD, 2);
+}
+
----------------
I find LE_BHW and LE_DBL to be pretty incomprehensible names.  For consistency and understanding, I would name them LE_WORD_0 and LE_DWORD_0.  Also, LE_CPW is arguably LE_WORD_1 and LE_CPD is LE_DWORD_1.  This gives the more understandable:

  dag LE_DWORD_1 = (v2i64 (COPY_TO_REGCLASS BE_DWORD_0, VSRC));

reinforcing that LE doubleword 1 is the same as BE doubleword 0.

I don't much care what you call LE_MVW so long as it is readable and implies moving an integer to a vector register.  Longer meaningful names beat brief incomprehensible ones.  Of course that's still better than long incomprehensible ones. ;)

================
Comment at: test/CodeGen/PowerPC/p8-scalar_vector_conversions.ll:74
@@ +73,3 @@
+; CHECK-LE: xscvdpspn [[REG1:[0-9]+]], 1
+; CHECK-LE: xxsldwi {{[0-9]+}}, [[REG1]], [[REG1]], 1
+}
----------------
I was confused by this at first, but now I get it.  In all of these tests, the code generation that you are checking for is just for the %splat.splatinsert calculation, right?  This is the part that translates into a scalar_to_vector node.  I assume that the generated code follows this up with code to splat element 0 to the entire result register, correct?


Repository:
  rL LLVM

http://reviews.llvm.org/D11471







More information about the llvm-commits mailing list