[PATCH] D140840: Tests + Improve cases for optimizing out some icmp(binop) patterns (mostly mul)
Noah Goldstein via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list