[PATCH] D152838: [ValueTracking] Add range computation from dominating conditions

[Aleksandr Popov via Phabricator via llvm-commits]

aleksandr.popov marked 4 inline comments as done.
aleksandr.popov added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6039
+static ConstantRange computeRangeFromCond(const Value *V, Value *Condition) {
+  if (match(Condition, m_LogicalAnd(m_Value(), m_Value()))) {
+    auto LHSR =
----------------
goldstein.w.n wrote:
> logicalor aswell?
I've added a comment that here we compute value's range from the conditional expression which must be true to reach this value. Thus, we process LogicalAnd only



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6061
+    if (LHS == V && match(RHS, m_APInt(C)))
+      if (C->getBitWidth() == BitWidth)
+        return ConstantRange::makeExactICmpRegion(Pred, *C);
----------------
goldstein.w.n wrote:
> goldstein.w.n wrote:
> > All of the above can be:
> > ```
> > CmpInst::Predicate Pred;
> > const APInt * C;
> > if(match(Condition, m_c_ICmp(Pred, m_Specific(V), m_APInt(C))) && C->getBitWidth() == BitWidth)
> >    return ConstantRange::makeExactICmpRegion(Pred, *C);
> > ```
> `m_c_ICmp` is also probably not needed b.c we cannoniclize constants to the RHS.
Thanks, good point!


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6102
+    // block.
+    BasicBlockEdge Edge(BI->getParent(), BI->getSuccessor(0));
+    if (Edge.isSingleEdge() && DT.dominates(Edge, CxtI->getParent())) {
----------------
goldstein.w.n wrote:
> Should this iterator through successors?
This check can be expensive, thus we execute it only for potentially profitable conditions: when the condition must be true to reach the value.



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

https://reviews.llvm.org/D152838