[PATCH] D101235: [InstCombine] ctpop(rot(X)) -> ctpop(X)

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added a comment.

In D101235#2714836 <https://reviews.llvm.org/D101235#2714836>, @lebedev.ri wrote:

> In D101235#2714835 <https://reviews.llvm.org/D101235#2714835>, @xbolva00 wrote:
>
>> In D101235#2714818 <https://reviews.llvm.org/D101235#2714818>, @lebedev.ri wrote:
>>
>>> @nikic please don't blindly stamp @xbolva00 reviews.
>>> The QC on those is the same as it ever was.
>>
>> I fixed UB, sorry. Fixed. I dont build llvm with msan. Great to know you never do mistakes :)
>
> It's a sign of lack of most basic negative test.
>
> I'm just really surprised to *still* see such basic teething problems
> after so many years of patches to this area of codebase,
> so i'm not really sure how to even suggest to please pay just a bit more attention. :/



In D101235#2714836 <https://reviews.llvm.org/D101235#2714836>, @lebedev.ri wrote:

> In D101235#2714835 <https://reviews.llvm.org/D101235#2714835>, @xbolva00 wrote:
>
>> In D101235#2714818 <https://reviews.llvm.org/D101235#2714818>, @lebedev.ri wrote:
>>
>>> @nikic please don't blindly stamp @xbolva00 reviews.
>>> The QC on those is the same as it ever was.
>>
>> I fixed UB, sorry. Fixed. I dont build llvm with msan. Great to know you never do mistakes :)
>
> It's a sign of lack of most basic negative test.
>
> I'm just really surprised to *still* see such basic teething problems
> after so many years of patches to this area of codebase,
> so i'm not really sure how to even suggest to please pay just a bit more attention. :/

Well, ctpop_add_no_common_bits contains ctpop( fshl(a, b, 16) ) so A != B path was triggered and still it was ok on my PC and some bots too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101235