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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
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));
----------------
lebedev.ri wrote:
> 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.
(to be most specific: the code to deal unops is still a specialization of the code
to deal with binops, they can be deduplicated further.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141877