[PATCH] D30445: [ValueTracking] Add a isIVNeverPoison helper

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: include/llvm/Analysis/ValueTracking.h:425
+  ///
+  /// The second component of the return value may be non-null only if
+  /// the first component is false.  If non-null, it is the poison
----------------
reames wrote:
> Hm, this interface seems likely problematic.  How does this handle the case where poison can be introduced by two different sources?  Do we need an iterator abstraction?  Or can we reliably just return null for those?
> 
> (If the later, please clarify in the comment.)
It is the latter, will fix the comment.


================
Comment at: lib/Analysis/ValueTracking.cpp:3917
+  auto *PN = cast<PHINode>(*PHIOpItr);
+  assert(PN->getParent() == L->getHeader() && "Precondition violated!");
+  if (PN->getIncomingValueForBlock(L->getLoopLatch()) != I)
----------------
reames wrote:
> What prevents this induction variable from being part of a longer phi cycle?  i.e. a indvar with conditional increments for instance?  i.e. this shouldn't be an assert of the documentation needs updated.
Good point -- at this layer it is probably best to just bail out in that case.


https://reviews.llvm.org/D30445