[PATCH] D38637: [InstSimplify] don't let poison inhibit an easy fold

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 11 19:11:48 PDT 2017


efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

My only potential concern here is that we could end up blocking optimizations because we're folding to undef rather than zero... but that's probably rare enough that it doesn't matter.  LGTM.



================
Comment at: lib/Analysis/ValueTracking.cpp:822-824
+  // If the shift amount could be greater than or equal to the bit-width of the
+  // LHS, the value could be undef, so we don't know anything about it. We have
+  // to be more conservative than the poison situation above.
----------------
spatel wrote:
> Oops - this comment doesn't make sense. An overshift produces poison too. Removing this check would mean we're going to fall through to the expensive check below more often though. Do we want to do that or should I just fix the comment?
IIRC the old version of this comment is just outdated; we recently adjusted LangRef to be a bit more aggressive with shifts because we have some transforms which depend on it being poison rather than undef.

Probably worth investigating getting rid of this at some point; I expect there are some interesting shifts we could analyze.


https://reviews.llvm.org/D38637





More information about the llvm-commits mailing list