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

[Jacob Yu]

pingshiyu wrote:

We want unit tests to protect against previous bugs, and also to serve as documentation of the expected behaviour.

For lowering passes of MLIR, bugs aren't caused by the lowering rewrite not being _applied_ properly, but because the rewrite pattern _itself_ is incorrect.

A unit test that applies a pattern and checks the resulting IR doesn't check the pattern itself is correct. This is what an integration/execution test would do. 

Going back to the example of `floordiv`, the _unit_ test goes something like:
```
// CHECK:   %[[QUOTIENT:.*]] = arith.divsi %arg0, %arg1 : i32
// CHECK:   %[[PRODUCT:.*]] = arith.muli %[[QUOTIENT]], %arg1 : i32
// ... etc
```
i.e. just the pattern itself - and this is the _only test_ that can be written for the lowering pattern.

If this is all the testing we have for the lowering, then the issues I see are:
- When the lowering gets updated, we have no way to know if _old bugs_ are triggered by this new lowering. We lose some benefits of unit testing in protecting against previous bugs.
- In fact, we don't even know what the old bug-triggering cases are, because every time the pattern gets updated the test changes too. We lose the benefit of documenting expected behaviour.
- Every case the developer wants to check has to be done manually, the CI/CD doesn't help them much compared to simply inspecting the lowering pattern.

An integration test would resolve the above issues for lowering passes.

I'm not proposing to change how LLVM transformations are developed. But for MLIR's _lowering passes_, there's a real need for integration tests like this - not to replace how unit testing is done for all transformations. 

There may be a way to do what I've described above as a unit test, in which case please let me know! Also, let me know if I wasn't clear anywhere :)

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