[PATCH] hasPath

[Chandler Carruth]

  just FYI follow-up...


================
Comment at: include/llvm/Analysis/CFG.h:52
@@ +51,3 @@
+/// Returns false only if we can prove that once 'From' has been executed then
+/// 'To' can not be executed. Conservatively returns true.
+bool isPotentiallyReachable(const Instruction *From, const Instruction *To,
----------------
Nick Lewycky wrote:
> Chandler Carruth wrote:
> > I think it would be useful to give some guidance about the performance (big-O complexity based) that users should expect to help guide callers.
> > 
> > I would also add a brief one-line comment.
> Great idea, I've written a chunk about the cost. I'd rather have something more like the complexity guarantees on something like the C++ standard, but ended up with a description of the algorithm. It'll do.
> 
> I tried and failed to write a \brief within 80-columns.
> 
You don't have to fit brief comments in 80-columns.

================
Comment at: include/llvm/Analysis/CFG.h:53
@@ +52,3 @@
+/// 'To' can not be executed. Conservatively returns true.
+bool isPotentiallyReachable(const Instruction *From, const Instruction *To,
+                            DominatorTree *DT = 0, LoopInfo *LI = 0);
----------------
Nick Lewycky wrote:
> Chandler Carruth wrote:
> > Would it be useful to include a convenience overload that works in terms of BBs?
> > 
> > Also, why not references to instructions? Can these usefully be null?
> It's uncommon to use references to IR in llvm. You're right that they can't be null, but I have an irrational dislike of this change. Sorry.
You replied to the second of my comments but not the first. ;] The first was the more important. Also, if your dislike is irrational, that says to me you should perhaps make the change.

================
Comment at: lib/Analysis/CFG.cpp:145-148
@@ +144,6 @@
+  if (A->getParent() == B->getParent()) {
+    // The same block case is special because it's the only time we're looking
+    // within a single block to see which comes first. Once we start looking at
+    // multiple blocks, the first instruction of the block is reachable, so we
+    // only need to determine reachability between whole blocks.
+
----------------
Chandler Carruth wrote:
> I would hoist this into a static helper function which can then carry this comment.
?


http://llvm-reviews.chandlerc.com/D996