[PATCH] D71660: [ValueTracking] isKnownNonZero() should take non-null-ness assumptions into consideration (PR43267)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:623
+        return true;
+    }
+  }
----------------
xbolva00 wrote:
> nikic wrote:
> > lebedev.ri wrote:
> > > jdoerfert wrote:
> > > > < is also fine, however unlikely.
> > > Right, https://rise4fun.com/Alive/plslNp
> > As you went through the effort of supporting different predicates, possibly extend this to also handle non-zero constant? That is, not just `x > 0` but also `x > 100`. Something along the lines of...
> > 
> > ```
> > if (!match(Cmp, m_ICmp(Pred, m_V, m_APInt(C))))
> >   return false;
> > 
> > // Possibly without the non-canonical predicates?
> > switch (Pred) {
> > case ICmpInst::ICMP_NE:
> >   return C->isNullValue();
> > case ICmpInst::ICMP_UGT:
> >   return true;
> > case ICmpInst::ICMP_UGE:
> >   return !C->isNullValue();
> > case ICmpInst::ICMP_SGT:
> >   return C->isNonNegative();
> > case ICmpInst::ICMP_SGE:
> >   return C->isStrictlyPositive();
> > case ICmpInst::ICMP_SLT:
> >   return !C->isStrictlyPositive();
> > case ICmpInst::ICMP_SLE:
> >   return C->isNegative();
> > }
> > ```
> Tests?
On a second thought, i don't really feel like trying to handle everything at once.
`X != 0` is the main motivation here.
Many other cases can be handled (https://rise4fun.com/Alive/6yR),
but i don't know that we need to special-handle them.
Do we know generic `KnownBits` (`computeKnownBitsFromAssume`)
does not handle them? I don't, and i won't until there are tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71660