[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:
> bcl5980 wrote:
> > 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.
> Seems to only match `m_Oneuse(m_Trunc(...))` so seems like only case is the truncate is entirely eliminated, no? Not sure I see how that could cause regressions.
What I mean is not IR level regression. Think about the case source type is i256, the dest type is i32. Remove the trunc and restore the next instruct to be i256 may cause extra instruction on backend. 


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

https://reviews.llvm.org/D143368