[Lldb-commits] [lldb] [llvm] [mlir] [DebugInfo] Add symbolic branches to DIExpression (PR #210850)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

> Also note that if this PR were to go forward, it would need to be broken up into smaller components: maybe just the IR component, then the MIR component, etc. It's also missing a producer for these (part of what I meant by "motivation").
> 
> I strongly recommend you write an RFC on the forums before going forward with this (if there is one already, please link it on the PR). For example, one question that comes to mind: why not re-use the existing OPs for this? If they are unused in their current form because it's impractical, can't we change them to use labels in the IR layer?

Since there seems to have been a stream of accepts here, and my post was missed, I wanted to make sure these questions were not lost:

1. Should we have had an RFC for this? It seems you have even more upcoming work, which would be nice to discuss beforehand? I suspect some of these were talked about in a round table last dev meeting, but still. An RFC would make clear the answer to 2 below, what the upcoming work is, whether we should expect an in-tree producer for these, etc. These answers would help evaluate whether this belongs in-tree.
2. Why not change the implementation of the existing attributes -- they seem largely unusable as they are -- instead of adding new ones? The end result of "offsets that are integers" are still compliant with the spec, the intermediate representation is irrelevant for this purpose.
3. You said that "About the only other patch split I could come up with was boilerplate and emission", what do you think of the split I suggested above? Separate IR and CodeGen patches are a very natural split.

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