[PATCH] D42086: [X86] Teach LowerBUILD_VECTOR to recognize pair-wise splats of 32-bit elements and use a 64-bit broadcast

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:8118-8127
+    SmallVector<SDValue, 4> Ops(2);
+    bool CanSplat = true;
+    for (unsigned i = 0; i != NumElems; ++i) {
+      if (!Ops[i % 2])
+        Ops[i % 2] = Op->getOperand(i);
+      else if (Ops[i % 2] != Op->getOperand(i)) {
+        CanSplat = false;
----------------
Initialize the first 2 elements to simplify the code?

```
    SmallVector<SDValue, 4> Ops({Op->getOperand(0), Op->getOperand(1)});
    bool IsSplatPair = true;
    for (unsigned i = 2; i != NumElems; ++i) {
      if (Ops[i % 2] != Op->getOperand(i)) {
        IsSplatPair = false;
        break;
      }
    }

```
I think this would also be easier to read if it was split off into a helper function / lambda because you could just early return when you detect that it's not a splat pair.



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:8132
+      MVT EltVT = VT.isFloatingPoint() ? MVT::f64 : MVT::i64;
+      MVT NarrowVT = MVT::getVectorVT(ExtVT, 4);
+      // Create a new build vector and cast to v2i64/v2f64.
----------------
The VTs are confusingly named. ExtVT is the current vector element VT (way back at line 7891). Can we rename things to make this clearer as a preliminary clean-up (ExtVT -> EltVT)? 


https://reviews.llvm.org/D42086