[Mlir-commits] [mlir] [mlir][arith] Add more canonicalization and integration tests coverage (PR #92272)

[Andrzej Warzyński]

banach-space wrote:

Thanks for sharing these, @pingshiyu !

I'm always in favour of improving our test coverage, but if we intend to add much more then it would be good to have some plan.

Another question that I'd ask - how far do we go with testing? Do we want to exercise every possible combination? For example, looking at "test-int-trunc-ext.mlir" that you added, there are the following 3 cases:
*  `@extsiOnI1`
* `@extuiOn1I1`
* `@trunciI16ToI8`

Why not more, i.e. other data types and operations?  Once we consider all the possibilities, the potential set of all tests becomes really large. This can be a maintenance burden. Are we worried?

Also, if we are to add _many_ more tests, it would be good to establish some structure so that it's easy to identify cases that are covered and to find cases that are missing. Otherwise, we tend to duplicate tests, I've seen examples in the Vector dialect, not sure about Arith. This can usually be improved with comments and consistent test function naming.

> Are we happy with the regression test format right now (i.e. either as canonicalization tests, or as integration tests)? If so I'll add the rest of the tests into the PR too.

I find integration tests very helpful, but they tend to be expensive and hence hidden behind a CMake flag. Improving the coverage for Arith would be a welcome addition, but it would be good to have a strategy for selecting good test cases. To me, this is a very good reference:
* https://github.com/llvm/llvm-project/blob/main/mlir/test/Integration/Dialect/Complex/CPU/correctness.mlir

IIUC, there's a case for every operation and every Op is verified with a set of inputs. I know that @Lewuathe has recently contributed there - would you have any recommendation for good cases to test?

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