Useful in debugging.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
| llvm/include/llvm/Analysis/LoopInfoImpl.h | ||
|---|---|---|
| 339 ↗ | (On Diff #65541) | Not sure if this needs to be a completely separate method -- why not just pass in a verbose flag to print? |
| 349 ↗ | (On Diff #65541) | Hm, just noticed that this is misleading both here and in the print method above -- if the loop has multiple latches, we won't tag any blocks as <latch>. This should be fixed (we need a isLoopLatch helper like we have isLoopExiting). @hiraditya - do you mind fixing this in a later change? If you won't be able to get to it, please let me know and I'll do it. |
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.
| llvm/include/llvm/Analysis/LoopInfo.h | ||
|---|---|---|
| 171 ↗ | (On Diff #65579) | Don't repeat function names in new doxygen comments. |
| 176 ↗ | (On Diff #65579) | I'd do this slightly differently. I'd have the precondition be that auto PredBegin = ...; auto PredEnd = ...; return std::find(BB, PredBegin, PredEnd) != PredEnd; |
| llvm/lib/Analysis/LoopInfo.cpp | ||
| 392 ↗ | (On Diff #65579) | Add a /*Verbose=*/ before true and /*Depth=*/ before 0. |
Split in two patches and addressed all last comments:
https://reviews.llvm.org/rL276837
https://reviews.llvm.org/rL276838
Thanks Sanjoy for your review.