[Mlir-commits] [mlir] [mlir][Arith][EmitC] Bail-out on arith.constant conversions to EmitC (PR #201565)

[Gil Rapaport]

aniragil wrote:

> Indeed. The code change is meant to make the convert self-document that converting arith.constant defining MemRef(s) is not supported.

So the description is wrong:

*Update ArithToEmitC to bail-out before creating invalid EmitC ops for unsupported cases.
ArithToEmitC now avoids rewriting memref constants to emitc.constant, since the type conversion changes the result type but the attribute cannot be converted into a valid EmitC initializer.*

> Test changes are meant to document the same through ... tests. You are right that the code change is not strictly required.

Adding tests to capture and document current state is (always) great, but the added code is dead ... if the purpose is to explicitly block memref types for constants regardless of what they'd be converted to, it should go above the type conversion.

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