[PATCH] D22817: Utility to print all the basic blocks of a loop.

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

LGTM with comments inline.

However: if it isn't too much extra work, can I ask you to please land
the `Verbose` flag in a separate change from the `isLoopLatch` change?
If that's too much extra work, don't bother, since these are low risk
changes.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:171
@@ -170,1 +170,3 @@
 
+  /// isLoopLatch - Returns true if \p BB is a loop-latch.
+  /// A latch block is a block that contains a branch back to the header.
----------------
Don't repeat function names in new doxygen comments.

================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:176
@@ +175,3 @@
+  bool isLoopLatch(const BlockT *BB) const {
+    BlockT *Header = getHeader();
+    typedef GraphTraits<Inverse<BlockT*> > InvBlockTraits;
----------------
I'd do this slightly differently.  I'd have the precondition be that
`BB` belongs to the loop (and start with an `assert(contains(BB))`),
and have the body just be a:

```
auto PredBegin = ...;
auto PredEnd = ...;
return std::find(BB, PredBegin, PredEnd) != PredEnd;
```

================
Comment at: llvm/lib/Analysis/LoopInfo.cpp:392
@@ +391,3 @@
+LLVM_DUMP_METHOD void Loop::dumpVerbose() const {
+  print(dbgs(), 0, true);
+}
----------------
Add a `/*Verbose=*/` before `true` and `/*Depth=*/` before `0`.


https://reviews.llvm.org/D22817