[PATCH] D142038: [X86] canonicalizeShuffleWithBinOps - add handling of SSE vector shift nodes

Thu Jan 19 03:40:23 PST 2023

RKSimon added a comment.

I'm sorry if I've caused you an issue here, but you did ask on D141877 <https://reviews.llvm.org/D141877> that I show you my alternative approach, and I did my best to get something to you as fast as possible, despite me explaining that I'm stretched at the moment.

The level of refactoring that you employ in your version is more than is needed to add support for the SSE shifts to fix the regressions on D141778 <https://reviews.llvm.org/D141778>, and I don't agree that generalizing the code so much is necessary, especially as it affects understand-ability.

Maybe if you proposed refactoring patches that weren't part of a dependency chain for a simple improvement then it'd be easier to have a conversation.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142038