[PATCH] D25912: [PowerPC] Improvements for BUILD_VECTOR Vol. 1

[Kit Barton via llvm-commits]

kbarton added a comment.

Can you please add some more comments in the summary about the intention of this group of patches?



================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:7203
+      return SDValue();
+
+    // SDAG patterns are provided for building vectors out of values that are
----------------
Would it be better to add this check in the condition above? 
Then at least the remaining logic might be able to lower this, instead of just punting to the default case. If not, then I think a comment here is justified. 


================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:7267
+    // elements are the same constant.
+    if (HasAnyUndefs || ISD::isBuildVectorAllOnes(BVN)) {
+      SmallVector<SDValue, 16> Ops(16, DAG.getConstant(SplatBits,
----------------
I don't follow this logic. Doesn't the isBuildVectorAllOnes already account for any Undefs in the vector? 


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:1383
 
-let UseVSXReg = 1 in {
+let UseVSXReg = 1, AddedComplexity = 400 in {
 let Predicates = [HasDirectMove] in {
----------------
Is there a way we can do this without the use of AddedComplexity?


================
Comment at: lib/Target/PowerPC/PPCInstrVSX.td:2752
+  let Predicates = [HasDirectMove] in {
+    /* Endianness-neutral constant splat on P8 and newer targets. The reason
+       for this pattern is that on targets with direct moves, we don't expand
----------------
Please convert to C++ comments


Repository:
  rL LLVM

https://reviews.llvm.org/D25912