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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

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

> This patch is quite difficult to review in it's current state. There are over 20 if statements with various levels of nesting in one function, with the largest if statement spanning 150 lines. I think some of these cases could be handled better with helper functions, where you could pass the binary operators through and deduce if the flags and operands are valid for a combine from within those functions. It's very difficult to keep track of which variables are which when they are named A, Y, BO1, CY, CZ, AY, AZ in combination with the long and layered if statements with verbose comments.

I'll cleanup the comments for V2 (and can also try and make the variable names more intuitive, thinking with `A` prefix -> `APint`, `C` prefix -> constant), but have trouble seeing how to add a useful helper. Each case is pretty specific both in the flags you need to check and the flags we can deduce for the output.

Where you thinking a helper for each case? Or something more generic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143014