[PATCH] D97046: [lit] Generalize `early_tests`

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/test/Unit/lit.cfg.py:17
+# start_phase; Request to start this suite early.
+config.start_phase = -1
 
----------------
davezarzycki wrote:
> mehdi_amini wrote:
> > What is the motivation for MLIR Unittests to start early?
> > I'd expect this to be at least documented. The comment here is just repeating *what* the code is doing instead of *why* we're doing this.
> I don't know. Presumably some of them are slow. If you're really curious, then I'd recommend digging through the SCM history to see why the original `is_early` flag was added.
This was just copied from LLVM as-is.

This kind of goes into my point about maintainability: the `is_early` was considered "hacky" (per author's words: https://reviews.llvm.org/D18089 ) but was coarse grain and minimalistic.  I'm not sure that the finer grain control introduced here goes in the same spirit.

So to make it clear: I don't think this feature is super intrusive to lit and the code is fine. It is more the "how it will be used in the repo" that makes me pause here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97046