[Mlir-commits] [mlir] [MLIR][LLVM] Add DILocAttr for debug locations (PR #186146)

[Tobias Gysi]

gysit wrote:

> I'm not sure I agree with this - the inliner can continue to produce callsite locations, just the caller/callee will be DILocAttr rather than FusedLoc. I would expect that we'd have some test failures if this change was that broken :)

I believe the tests will start failing once LocationAnnotationAttr is updated. The existing inlining tests still use the FusedLoc which should be changed.

> To me there are currently 2 issues with the representation...

These are good arguments for a brief RFC or the PR description. It may also make sense to explain that the inlining information will still be modeled using the MLIR way (at least this is my understanding).

> +1, seeing the context of how this would be used I think will help.

I am glad we agree. 

As LLVM dialect maintainer I want to make sure the migration does not stop half way in and we are left with multiple representations for the same thing. The proposed change is relatively small and we should make sure to fully finish it (which includes the relevant changes to the LoopAnnotationAttr and the in-tree projects that produce location attributes).

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