search

tensorflow / mlir

"Multi-Level Intermediate Representation" Compiler Infrastructure
1.74k stars 260 forks source link

-lower-to-llvm/mlir-cpu-runner: missing line number info in errors #48

Open bondhugula opened 5 years ago

bondhugula commented 5 years ago

$ mlir-opt -lower-to-llvm test/Transforms/dma-generate.mlir

:0: error: only 1D vectors are supported I was expecting line number information here to be available.
River707 commented 5 years ago

This is from the type converter, which isn't provided with a source location.

bondhugula commented 5 years ago

Isn't the type (that is being provided to the type converter) from a unique operation? In that case, can the source location be passed to it?

River707 commented 5 years ago

A majority of the types come from block arguments, which don't have an exact location. Though I suppose we could use the location of the parent operation of the region the block is contained in.

bondhugula commented 5 years ago

For block argument types, the source location is ideally expected to be the source location of the block label (and that of the parent operation if it's the entry block), but I think this information isn't maintained in MLIR (I don't see a block->getLoc(), and ran into this recently)? The parent operation's location is a reasonable approximation though.

In any case, the error observed here leaves the user clueless as to where the lowering failed.

lattner commented 5 years ago

I've struggled with this in SIL before, and have tried all the horrible hacks (none of which work well) - is there a disadvantage to adding a location to blockarguments? When (e.g.) mem2reg builds a block argument, it can easily propagate the location from the load.

bondhugula commented 5 years ago

Below is yet another case of missing line number info due to arguments missing such info.

$ mlir-cpu-runner -e test /tmp/test.mlir Error: non-memref argument not supported

allocMemRefDescriptor should be taking in a location argument: https://github.com/tensorflow/mlir/blob/77efdea3c70df3b68aeecbbc82ee9e969dca499c/lib/ExecutionEngine/MemRefUtils.cpp#L39