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

[Jacob Yu]

pingshiyu wrote:

@joker-eph 
> Right, but that's "separation of concerns". It's all what unit-testing is about: we check for a component boundaries based on the expectation at these boundaries.
I do agree that the scope of tests should be limited, but I think where we differ is what the "expectation" should be.
Take the example of the lowering for instance: we want the test to give us confidence that translated code is "correct".
https://github.com/llvm/llvm-project/pull/83248/files#diff-71577b98cdb92d5eb7583c3c3d5b9488f017f8f56dc6d05b7b940848f6e0bfc9
However, I don't think checking that "the translated code is equal to what we expect" gives us confidence in this case - it is only checking the rewrite pattern has done its job. It is not checking that the lowering is correct, which I think is very desirable too for the pass. I don't think the linked unit-test, by itself, gives us much confidence that the bug has even been fixed: we'd need to run the code (mentally, or via interpreter) on the problem input described in the bug report to be sure. 

I think it's clearly desirable to test the functionality of the lowering, though. And I don't see how this can be done without something to the effect of an integration test.

> I personally don't see a different between the expand-math in MLIR and the same thing on LLVM IR though.
So for me, my understanding is that LLVM optimisations can be considered a rewrite within the same abstraction layer, i.e. an optimisation pass. 
The "correctness" of an optimisation is that a program pattern has been _applied_ correctly. There are many source patterns the optimisation can be applied to, so we can test in the unit-test fashion by checking the rewrite is correct on small examples.
On the other hand, lowerings are mappings of single operations to a pattern in the lower-level dialect - the "correctness" of a lowering is that the instructions has the same semantics as the lower-level pattern, which isn't being checked if we're only checking that the rewrite has been applied.

@banach-space the ones referenced in https://github.com/llvm/llvm-project/issues/100121 would be all the ops I intend to test in this work!

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