[PATCH] D141877: [NFC][X86] `canonicalizeShuffleWithBinOps()`: refactor and generalize, NFC

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

@RKSimon if you prefer, you may write these changes by yourself. I can abandon these patches.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:40498
+        std::array<SDValue, 2> N0Ops;
+        for (auto I : zip(N0->ops(), N0Ops))
+          std::get<1>(I) = peekThroughOneUseBitcasts(std::get<0>(I));
----------------
RKSimon wrote:
> Given we know that N0 is a binop - all this generic zip/enumate iteration code seems unnecessary tbh.
This is a generalization. Currently, the code does exactly the same,
but the implicit loop is manually unrolled. With unrolled loops,
you suffer the cost of duplicate LOC, having double the LOC to verify for correctness,
and verify that they do the right thing for right "iteration" (operand).
Doing that is counter-productive and anti-general.

In fact, that is the whole reason why LLVM is seeing such an explosion of code size,
because everything is special-coded, instead of having a "few" general utils,
that are both easy to prove to be correct, and don't incur bloat,
and are easy to use, and make easier to write new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141877