[PATCH] D63390: [Codegen] TargetLowering::SimplifySetCC(): omit urem when possible

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

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

> Thank you for the reviews.
>
> In D63390#1546676 <https://reviews.llvm.org/D63390#1546676>, @spatel wrote:
>
> > LGTM. I have no idea if it's worth the trade-off, but we could do this sooner in IR (instcombine) instead of or in addition to SDAG?
>
>
> Indeed, this fold is missed in middle-end too, but the regression at hand is in back-end test,
>  so i'm not sure if we should just hand-wave and only fix it in middle-end.
>  Also, where should the middle-end fix be? Again InstCombine?


Yep, yet another clause under InstCombiner::visitICmpInst().

> I'm also not sure if this is the best approach, the fold doesn't care *where* those bits are,
>  it only cares about the *count* of ones, and is thus easily defeated by shift operations,
>  as it is seen from tests.

True, although without some real-world evidence that this matters, I'd say "good enough". :)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63390