[Mlir-commits] [mlir] [mlir] Use MLIR op names when generating FileCheck variables in generate-test-checks.py (PR #160820)

[Andrzej Warzyński]

banach-space wrote:

Thanks for taking a look and for the feedback!

> I concerned about this because the script is not meant to provide something readable in a "ready to use" kind of way.

Note, the high-level rationale behind this script does not change: https://github.com/llvm/llvm-project/blob/0d989b2aefe8d8f66981f1d39f56ce0214b5de86/mlir/utils/generate-test-checks.py#L22-L23

> I personally strongly object to having tests are not minimally checking the output

Agreed - that’s exactly what we document under [FileCheck Best Practices](https://mlir.llvm.org/getting_started/TestingGuide/#filecheck-best-practices). This PR doesn’t change that principle; it only tweaks how placeholders are named.

> Making the script better here may be counter-productive in that it would be encouraging people to not pay more attention to the naming. 

To me, moving from `VAL_0` to something like `ADDI_0` is a small step in the right direction, and probably sufficient in many cases. Ultimately, it’s contributors and reviewers who make sure our [Testing Guide](https://mlir.llvm.org/getting_started/TestingGuide/?utm_source=chatgpt.com) is followed.

Totally agreed that no script or automation should replace human input. Personally though, I’d much rather see `[[ADDI_0]]` than `[[VAL_0]]` when the human input is skipped. 

Btw, I’ll also highlight the Testing Guide at LLVM Dev to reinforce it.

_Just a heads up: I’ll be offline most of next week, so my responses will be slower than usual._

https://github.com/llvm/llvm-project/pull/160820