[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