[PATCH] D31373: [X86][SSE]] Lower BUILD_VECTOR with repeated ops as BUILD_VECTOR + VECTOR_SHUFFLE

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6112-6113
 
+// Attempt to lower a build vector of repeated ops as a build vector of unique
+// ops followed by a shuffle.
+static SDValue
----------------
"build vector of repeated ops" translated to "splat" in my mind when I read this. I think we guarantee that case won't make it this far, so assert that condition?
How about "build vector with repeated ops (but not a full splat)"?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6115
+static SDValue
+lowerBuildVectorAsInsertAndPermute(SDValue V, SelectionDAG &DAG,
+                                   const X86Subtarget &Subtarget) {
----------------
I'd rather not use "Permute" in the name here since that implies one of those specific AVX instructions. "lowerBuildVectorWithRepeatedEltsUsingShuffle"?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6129
+  // might be better to insert them in other locations for shuffle efficiency.
+  bool RepeatedElts = false;
+  SmallVector<int, 16> Mask(NumElts, SM_SentinelUndef);
----------------
I prefer to put a verb on these kinds of bools - "HasRepeatedElts"?



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6138
+
+    // Zeros can be efficiently repeated, don't shuffle these.
+    if (X86::isZeroNode(Op))
----------------
Run-on: "repeated, so don't"


Repository:
  rL LLVM

https://reviews.llvm.org/D31373