[Mlir-commits] [mlir] [mlir][EmitC] Convert math::RoundEvenOp, SqrtOp and RsqrtOp (PR #190158)

[Marius Brehler]

https://github.com/marbre approved this pull request.

Thanks for elaborating @banach-space!

> We could, however, convert to C++ and then compile to make sure that everything works as expected. Basically, instead of:
> 
> ```shell
> mlir-opt file.mlir | mlir-runner
> ```
> 
> (that's what we use for "runnable" tests) we could run this for EmitC:
> 
> ```shell
> mlir-opt file.mlir | mlir-translate -mlir-to-cpp | clang -Wall -Werror -fsyntax-only -c
> ```
> 
> I don't have a strong opinion, there aren't that many tests involved. I guess that it is also a matter of where EmitC is heading and what's the right testing strategy. From our perspective the priority is to make sure that we can lower from our TOSA models to EmitC and that nothing regresses. IMHO, transformation and conversion tests are insufficient for this.
> 
> WDYT?


I think the latter would be valid and is what we had done in https://github.com/iml130/mlir-emitc. From my understanding the integration tests should indeed be executable. If we intend to call clang (or gcc), we should eventually discuss this in an RFC? For now fine as is but maybe worth a comment in the test file though. Other than that looks good to me.

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