[PATCH] D154984: [lit] Drop "Script:", make -v and -a imply -vv

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added a comment.

In D154984#4493934 <https://reviews.llvm.org/D154984#4493934>, @jdenny wrote:

> After the points you raise, I agree we should have an RFC before landing this patch.  However, do you mind if we have more discussion here first?  I think that would produce a better RFC.

Sure! Just to clarify my view:

- strong +1 to how the relation between `-v`/`-a`/`-vv` are cleaned-up in this patch,
- preserving "Script" would be helpful.

I rely on "Script" when debugging test set-ups. Specifically, MLIR integration tests (e.g. sparse_conversion.mlir <https://github.com/llvm/llvm-project/blob/60c9d2993bbf1594e89e1e6f72e1472eb1aeb8ef/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_conversion.mlir#L1-L27>) that can:

- run natively or via an emulator,
- use host or x-compiled `lli` (or `mlir-cpu-runner`),
- run 3-4 tools per RUN line (i.e. `tool-1 | tool-2 | tool-3 | tool-4`).

The resulting `RUN` lines tend to be quite complex. Having them summarised in one place helps.

For reference, this is what I get with "Script" (ToT LLVM):

  : 'RUN: at line 10';  <WORKDIR>build/release/bin/mlir-opt <WORKDIR>/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_index.mlir --sparse-compiler=enable-runtime-library=true | <WORKDIR>/build/release/bin/mlir-cpu-runner -e entry -entry-point-result=void -shared-libs=<WORKDIR>/build/release/lib/libmlir_c_runner_utils.so | <WORKDIR>/build/release/bin/FileCheck <WORKDIR>/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_index.mlir

This can be c&p into a shell and will "just work". That's very useful.

This is what I get with `-a` with your patch:

  + <WORKDIR>/build/release/bin/mlir-opt <WORKDIR>/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_index.mlir --sparse-compiler=enable-runtime-library=true
  + <WORKDIR>/build/release/bin/mlir-cpu-runner -e entry -entry-point-result=void -shared-libs=<WORKDIR>/build/release/lib/libmlir_c_runner_utils.so
  + <WORKDIR>/build/release/bin/FileCheck <WORKDIR>/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_index.mlir

Sadly, this won't work in a shell without a bit of formatting. Could this be updated to match "Script"? If yes then that's not a big issue. I would still like for "Script" to be preserved, but that's not a blocker for me (I would still suggest checking on Discourse). Btw, IMHO it would be helpful to include the ideas proposed in https://reviews.llvm.org/D154987 in any potential RFC.

Last, but not least, thank you for caring about LIT and improving it for us - that's greatly appreciated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154984