[PATCH] D140840: Tests + Improve cases for optimizing out some icmp(binop) patterns (mostly mul)

Mon Jan 2 11:33:21 PST 2023

goldstein.w.n added a comment.

In D140840#4021782 <https://reviews.llvm.org/D140840#4021782>, @nikic wrote:

> In D140840#4021722 <https://reviews.llvm.org/D140840#4021722>, @goldstein.w.n wrote:
>
>> In D140840#4021706 <https://reviews.llvm.org/D140840#4021706>, @nikic wrote:
>>
>>> This patch is mixing up a lot of different changes. Please split it up into self-contained changes to the degree that this is possible. I would recommend starting with just your changes in InstCombineCompares, as these look like they should be able to stand on their own (without changes to AssumptionCache machinery).
>>
>> Sure, should I just post back to this one with the changes split into more commits or create a new review starting with
>> the InstCombineCompares change?
>
> Locally you'd probably want to split into commits, but as far as Phabricator is concerned they should be separate reviews -- Phabricator does not allow reviewing individual commits in a review. I don't know how one does that through `arc`, I always upload patches via the web interface.

Okay, I split it into 4 commits and 4 separate reviews:

Tests
-----

https://reviews.llvm.org/D140849

InstCombineCompares
-------------------

https://reviews.llvm.org/D140850

AssumptionCache
---------------

https://reviews.llvm.org/D140851

isKnownNonZero
--------------

https://reviews.llvm.org/D140852

Is there a way to close this review?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140840