[PATCH] D143368: [InstCombine] Look through truncate to fold icmp with intrinsics
chenglin.bi via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 8 18:43:29 PST 2023
bcl5980 added inline comments.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4918
+ if (auto *II = dyn_cast<IntrinsicInst>(X)) {
+ if (II->getIntrinsicID() == Intrinsic::ctpop ||
----------------
goldstein.w.n wrote:
> Imo it makes more sense to do this generically.
>
> `(icmp P (trunc(X), C))` if `KnownBits(X)[OrigWidth:TruncWidth] == 0` just drop the truncate. That will cover all these intrins + any other cases that happen to come up.
I don't want to enable all cases for the trunc. That may cause a lot of potential regressions. By default we should shrink bits if possible not expand it. I prefer to limited the change to the case we can really get improvement.
Maybe I can move this change to SDAG to avoid these concern.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4920-4921
+ if (II->getIntrinsicID() == Intrinsic::ctpop ||
+ II->getIntrinsicID() == Intrinsic::cttz ||
+ II->getIntrinsicID() == Intrinsic::ctlz) {
+ // Make sure the dest bits is enough to save the intrinsic output's range
----------------
spatel wrote:
> Require one less bit for cttz/ctlz when the "is_zero_poison" argument is true?
https://alive2.llvm.org/ce/z/pQQavA
It looks one less bit for cttz when is_zero_poison=true is wrong.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143368/new/
https://reviews.llvm.org/D143368
More information about the llvm-commits
mailing list