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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

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.



================
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.
----------------
s/`I`/`BB`/


================
Comment at: unittests/Analysis/LoopInfoTest.cpp:103
+  // Parse the module.
+  LLVMContext Context;
+  std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleStr);
----------------
You can reduce some of this boilerplate by a `runWithLoopInfo` helper, as in: https://github.com/llvm-mirror/llvm/blob/master/unittests/Analysis/ScalarEvolutionTest.cpp#L54


https://reviews.llvm.org/D28756