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

Ehud Katz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 6 12:06:11 PST 2020


ekatz marked an inline comment as done.
ekatz added inline comments.


================
Comment at: llvm/test/Transforms/CodeExtractor/LoopExtractor_crash.ll:4
+
+; This test used to trigger an assert.
+
----------------
fhahn wrote:
> ekatz wrote:
> > 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.
> Sure, but it would still be good to check that the pass does what we expect (not crashing is a relatively low bar). I guess this is a case where the function is just a minimal container? If that's the case, that's worth explicitly checking (and probably best to also add a short comment & a reference to the bug).
> 
> also without check, the pass could also delete the whole function in order not to crash ;)
I agree. Done.


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

https://reviews.llvm.org/D69069





More information about the llvm-commits mailing list