[PATCH] D143368: [InstCombine] Look through truncate to fold icmp with intrinsics

[chenglin.bi via Phabricator via llvm-commits]

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