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

[Scott Linder]

slinder1 wrote:

> I still don't necessarily agree that this is a required step for something usable as it changes nothing about DIExpression to use. If you want to add on such facilities later we can talk about how to design those in.

I think this _does_ change DIExpression, though. As-is the change impacts (in no particular order):

* Future work on `entry_value`
* Future work on optimizing `DIExpression`s
* The actual code around `DIExpression`, to the point of adding a whole feature-conflict-resolution aspect to `isValid`. Anyone working on any of this code in the future will need to consider control-flow.
* The soundness of code which already edits `DIExpression`s

I think @Snowy1803 gave a perfect example for that last point:

> What happens if a frontend creates a DIExpression with branching and without an arglist, and then salvageDebugInfo, through an optimization, rewrites the argument to use an arglist when a binary instruction is deleted? Would it just crash? I think that if an expression can't be lowered due to an implementation detail, the debug value should just be killed. Hopefully support for DIArgList can be added later.

I think in this case we will either `assert` or the verifier will strip _all_ debug-info for the module. Neither seems like what we want in a case I expect will immediately crop up in any real use of the feature.

IMO it is a stretch to claim adding this has no impact on `DIExpression`, when the only way to achieve that is to so underdefine the feature as to allow the compiler to effectively just drop any expression using it at any point.




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