[PATCH] D123494: [VectorCombine] Find and remove shuffles from commutative reductions

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:1162
+    if (auto *CI = dyn_cast<Instruction>(CV)) {
+      if (CI->isBinaryOp()) {
+        for (auto *Op : CI->operand_values())
----------------
samtebbs wrote:
> Does this also need to check if the binary operation is commutative? It may be a good idea to add a test with a non-commutative reduction and a qualifying shuffle, to make sure this function's behaviour isn't broken in the future.
I don't think it should matter if the binary operator is commutative. There is a test with a lshr in reduceshuffle_twoin_ext_v16i32, for example. These binary operators we are only looking through, back to the shuffle. We don't modify them directly, just the order that the vector-lanes operate. Which is safe because they are only used by the reduction, and only use splats and the shuffle we transform.


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

https://reviews.llvm.org/D123494