[PATCH] D69069: [LoopExtractor] Convert LoopExtractor from LoopPass to ModulePass

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 6 04:12:46 PST 2020


fhahn accepted this revision.
fhahn added a comment.

LGTM, with comments addressed before committing. Thanks!



================
Comment at: llvm/lib/Transforms/IPO/LoopExtractor.cpp:151
+
+  // If LoopSimplify form is not available, stay out of trouble.
+  if (TLL->isLoopSimplifyForm()) {
----------------
nit: the comment is a bit confusing, as it checks if the loop is in simplify-form. Maybe say something like: If the loop is in LoopSimplify form, check if the function is a minimal container around the loop.


================
Comment at: llvm/test/Transforms/CodeExtractor/LoopExtractor.ll:3
+
+; CHECK: define
+; new 2 outlined function
----------------
I think it would be good to check the full IR after extraction.


================
Comment at: llvm/test/Transforms/CodeExtractor/LoopExtractor_crash.ll:4
+
+; This test used to trigger an assert.
+
----------------
There are no arguments to promote and no functions to inline? Is -inline/-argpromotion needed? Also, please add some rudimentary checks. (you could use llvm/tools/update_test_checks.py)


================
Comment at: llvm/test/Transforms/CodeExtractor/LoopExtractor_infinite.ll:3
+
+; This test used to enter an infinite loop (until out of memory).
+
----------------
Is running -mergereturn required? Or is it enough to have the output of merge return as test case? Also it would be good to have some rudimentary checks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69069/new/

https://reviews.llvm.org/D69069





More information about the llvm-commits mailing list