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

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 16 09:09:27 PST 2017


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





More information about the llvm-commits mailing list