[PATCH] D143014: [InstCombine] Add combines for `(urem/srem (mul/shl X, Y), (mul/shl X, Z))`

Noah Goldstein via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 13 19:38:39 PST 2023


goldstein.w.n added a comment.

In D143014#4115845 <https://reviews.llvm.org/D143014#4115845>, @MattDevereau wrote:

> I think not implementing several of my suggestions because of a future patch is a mistake. I don't think they're nits and have some obvious benefits for readability and control flow, and I'm of the opinion that leaving code that anticipates future work that may or may not even land or be reverted is not ideal, so I'll leave this for someone else to approve.

Sorry, missed this earlier, just seeing now.

If you insist, will update accordingly. Think this version has all your concerns addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143014



More information about the llvm-commits mailing list