[PATCH] D28756: Rewrite part of how loop ID is obtained.

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 16 09:28:11 PST 2017


hfinkel added a comment.

In https://reviews.llvm.org/D28756#647380, @trentxintong wrote:

> In https://reviews.llvm.org/D28756#647368, @sanjoy wrote:
>
> > Code-wise this looks fine, but the "If a loop has multiple backedges and all of them have the same metadata we return that metadata. Otherwise we return nullptr." bit should be documented in the LangRef.  I'll also ask you to please wait for someone familiar with how LLVM uses `!llvm.loop` to give a final go head before checking this in.
>
>
> Thanks for the review. I will wait for @hfinkel to give the final approval.


I think that this makes sense. I agree with Sanjoy that we should make sure that the LangRef reflects what we actually do here.



================
Comment at: lib/Analysis/LoopInfo.cpp:223
+    assert(H && "Loop must have a header");
+    for (auto I = pred_begin(H), E = pred_end(H); I != E; ++I) {
+      // Ignore header's predecessor outside the loop.
----------------
sanjoy wrote:
> s/`I`/`BB`/
You should be able to use a range-based for here, something like:
  for (auto &BB : predecessors(H)) {


https://reviews.llvm.org/D28756





More information about the llvm-commits mailing list