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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n 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 =
----------------
logicalor aswell?


================
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);
----------------
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);
```


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6083
+  unsigned NumBlocksExplored = 0;
+  DomTreeNode *Node = DT.getNode(CxtI->getParent());
+  while (Node = Node->getIDom()) {
----------------
Think you need to check that Node != nullptr.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6089
+
+    assert(Node->getBlock());
+    const BranchInst *BI =
----------------
nit: assert message.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:6102
+    // block.
+    BasicBlockEdge Edge(BI->getParent(), BI->getSuccessor(0));
+    if (Edge.isSingleEdge() && DT.dominates(Edge, CxtI->getParent())) {
----------------
Should this iterator through successors?


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

https://reviews.llvm.org/D152838