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

[Ehud Katz via Phabricator via llvm-commits]

ekatz marked 3 inline comments as done.
ekatz added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/LoopExtractor.cpp:151
+
+  // If LoopSimplify form is not available, stay out of trouble.
+  if (TLL->isLoopSimplifyForm()) {
----------------
fhahn wrote:
> 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.
Done.


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


================
Comment at: llvm/test/Transforms/CodeExtractor/LoopExtractor_crash.ll:4
+
+; This test used to trigger an assert.
+
----------------
ekatz wrote:
> fhahn wrote:
> > 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)
> inline and argpromotion shouldn't do anything but certainly not crash. This crash is observed in those cases.
I don't think there is any reason to test the resulting IR. This test is only testing that there is no crash.


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

https://reviews.llvm.org/D69069