[PATCH] D102864: [InstSimplify] Transform X * Y % Y --> 0.

Fri May 21 12:53:43 PDT 2021

spatel added a comment.

In D102864#2774356 <https://reviews.llvm.org/D102864#2774356>, @davidtgoldblatt wrote:

> In D102864#2773521 <https://reviews.llvm.org/D102864#2773521>, @spatel wrote:
>
>> No problem. I can commit on your behalf. To confirm - use your gmail address for author info?
>
> Yep, that should be fine, thank you.

Please update and regenerate the test CHECK lines after: 3c4b79481d45 <https://reviews.llvm.org/rG3c4b79481d457b60a1c80fbdac5335be681a9dbe>

>> Would it make sense to unify this with the related div transforms? If I'm seeing it correctly, we could also pick up this (and unsigned sibling) transform if we can generalize that block:
>> https://alive2.llvm.org/ce/z/AvaDGJ
>
> I.e. pull out the signedness checks into `bool NoOverflow` or something, and additionally set `NoOverflow` to true if the X in X * Y / Y or X * Y % Y is of the form A / Y? I think we still need to check the specific operation (to decide to reduce to X or 0), but I think it does get us some extra cases we can simplify. Happy to do so if you'd prefer, although my initial thought is that it's a little complex for a case I don't know I've seen.

That's fine - we can make that a potential follow-up. You could add a TODO comment. The related block in simplifyDiv() is at line 1055. We make the code a bit more complicated by generalizing it in simplifyDivRem(), but then we're less likely to lose a transform.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102864