[PATCH] D82717: [InstSimplify] Fold icmp with dominating assume

[Nikita Popov via Phabricator via llvm-commits]

nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:3287
+                                               const SimplifyQuery &Q) {
+  if (!Q.AC || !Q.CxtI)
+    return nullptr;
----------------
spatel wrote:
> Would it make sense to also use "Q.AC.assumptions().empty()" as an early exit?
> We use that in InstCombiner::visitSelectInst() to mitigate cost for the presumed common case where there are no assumes available.
I believe the reason why visitSelectInst() checks for `assumptions().empty()` is that it does not directly query the assumption cache, but goes through a known bits calculation that takes assumes into account indirectly. In this case we query the assumption cache directly, so if there are no assumes we'll just perform a lookup in an empty hash table.

I wonder if that code in visitSelectInst() is needed at all, as GVN can also perform that fold... EarlyCSE can as well, but not in all cases (as it is conditioned on "SimpleValue::canHandle").


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

https://reviews.llvm.org/D82717