[PATCH] D74495: [VectorCombine] try to form vector binop to eliminate an extract element

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 4 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:105
+  Type *ScalarTy = I.getType();
+  Type *VecTy = X->getType();
+  int ScalarBOCost = TTI.getArithmeticInstrCost(BOpcode, ScalarTy);
----------------
lebedev.ri wrote:
> Don't you want to ensure that both `X` and `Y` have the same type?
> (we didn't forget that in `foldExtractCmp()`)
Yes - good catch. Accidentally dropped that check.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:135
+  }
+
+  // TODO: Handle C0 != C1 by shuffling 1 of the operands.
----------------
lebedev.ri wrote:
> Thinking out loud: what about the case where `X == Y`?
> * If the extract is of the same element, could consider doing extract from `2*X` or `X+X`
> * If elements are different, could consider forming `hadd`
For now (with same element extraction), it should work as expected. I'll add a test. I don't think we want to attempt canonicalization like X+X --> X<<1 if that's what you're thinking of. Let's leave that to instcombine.

With different elements, we'll want to be careful that we are not obscuring a pattern that the backend recognizes. I think we're ok on that either way for x86 horizontal math, but I'll take a closer look before the planned enhancement.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74495/new/

https://reviews.llvm.org/D74495