mlir-translate error + missing location pointing
On the appended MLIR, mlir-translate produces this error.
$ mlir-translate -mlir-to-llvmir /tmp/single.mlir loc("/tmp/single.mlir":1:6): error: non-LLVM function argument type
I see two issues:
- On a minor note, the error message is missing reproduction and pinpointing to the source line being referred to.
- It's not clear what the error is about. It looks like it's pointing to the '@' sigil (the sixth character), but that's not a function argument. Is this due to something missing with memref type lowering? The doc at https://github.com/tensorflow/mlir/blob/master/g3doc/ConversionToLLVMDialect.md doesn't say so.
func @simple_func(%arg0: memref<8x8xvector<64xf32>>, %arg1: memref<8x8xvector<64xf32>>, %arg2: memref<8x8xvector<64xf32>>) -> memref<8x8xvector<64xf32>> { affine.for %i = 0 to 256 { affine.for %j = 0 to 256 { affine.for %k = 0 to 250 { %l = load %arg0[%i, %k] : memref<8x8xvector<64xf32>> %r = load %arg1[%k, %j] : memref<8x8xvector<64xf32>> %o = load %arg2[%i, %j] : memref<8x8xvector<64xf32>> %m = mulf %l, %r : vector<64xf32> %a = addf %o, %m : vector<64xf32> store %a, %arg2[%i, %j] : memref<8x8xvector<64xf32>> } } } return %arg2 : memref<8x8xvector<64xf32>> }
On the other hand, doing the translation with mlir-opt -convert-to-llvmir doesn't have this issue. But it yields:
$ mlir-opt -convert-to-llvmir /tmp/single.mlir /tmp/single.mlir:2:3: error: 'affine.for' op expected body to have a single index argument for the induction variable affine.for %i = 0 to 256 {
I think the error message here is misleading (or perhaps a bug). The following however works since these 'for' op's are lowered away.
$ mlir-opt -lower-affine -convert-to-llvmir /tmp/single.mlir | mlir-translate -mlir-to-llvmir
Is mlir-translate missing documentation or is it out of date? It doesn't appear at https://github.com/tensorflow/mlir/blob/master/g3doc/ConversionToLLVMDialect.md and it's not clear what the difference b/w mlir-opt's -convert-to-llvmir and mlir-translate's -mlir-to-llvmir is.
mlir-translate is intended to translate MLIR to other IRs. Some of the translations require MLIR to only have certain dialects, in particular translation to LLVM IR requires MLIR input to only use the LLVM IR dialect. Hence the complaint about non-LLVM function argument types. None of them belongs to the LLVM dialect.
mlir-opt performs MLIR-to-MLIR transformations, including dialect conversions. The document you referenced is about converting to the dialect, not to the LLVM IR proper. The conversion is implemented for a subset of standard dialect. You need to lower away all other dialects before calling it. There is currently no way of triggering a chain of conversions that would somehow know that there is a pass that lowers away loops.
Right. This is behaving correctly. mlir-translate is just supposed to do the last step of translation (format conversion) it does not do lowering.
I see - thanks. In that case, shouldn't mlir-opt's '-convert-to-llvmir' be renamed - since it's converting to LLVM dialect as opposed to LLVM IR? Perhaps to '-convert-to-llvm'?
What about -lower-to-llvm or -llvm-lowering?
Even if issuing an error is expected here, the error messages should really be improved here, we should keep this bug open to track fixing this.
+1 for renaming to "lower-to-llvm" and improving the error messages!
Yes, even I wanted to point out that renaming sometime back. "lower-to-llvm" makes much more sense to me too.
The renaming was done in 822e3fc877e89dd7dc73d3018accdda6ea30b537
Thanks. The only thing remaining on this issue is this error message that probably has to be improved.
$ mlir-opt -lower-to-llvm /tmp/single.mlir /tmp/single.mlir:2:3: error: 'affine.for' op expected body to have a single index argument for the induction variable affine.for %i = 0 to 256 {
This should have ideally been a "can't lower 'affine.for' op".
Issues referenced here have been addressed, pending one is a request to rename which could best be done by way of a PR & discussion there, so closing this issue.
Actually the rename has been done, the remaining issue is about the lowering error message being confusing.