[PATCH] D92013: [DebugInfo] Don't use DW_OP_implicit_value for fragments

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

Thinking about this further, I am not sure if the approach with multiple alternatives is a good solution (for this particular problem at least). If we take x87 long double constants (currently not supported here, but one of the things I wanted to look at) for example, they are 10 bytes long. Representing them with DW_OP_implicit_value is trivial. However, if we wanted to represent them with DW_OP_const, we would need to break it down into smaller chunks, something like `DW_OP_const low64, DW_OP_stack_value, DW_OP_piece 8, DW_OP_const high16, DW_OP_stack_value, DW_OP_piece 2`. If the DIExpression is empty, then that's fairly easy. However, if the expression already contains some operators (DW_OP_LLVM_fragment in particular), then I fear things would get messy. How messy depends on what is actually allowed to be contained in these expressions, which brings me to my next question:

What kind of operands are actually allowed in an expression describing a constant? DW_OP_LLVM_fragment is obviously useful, but I'm not sure about the other operations. For example, the arithmetic operations could simply be performed by applying the desired operation directly to the constant. DW_OP_deref might be interesting in cases where one wants to dereference a constant pointer, but in that case, the constant already needs to be address-sized, so there should be no further fragmenting.

One possibility I considered is to change the code which creates fragments (I don't yet know where this code lives) in such way that it always fragments that don't need any further fragmenting. The complications I see with that are:

- it would create the reverse problem of having to jump through hoops to reconstitute the fragments, if we wanted to emit a `DW_OP_implicit_value` (maybe we don't have to do that?)
- the fragmenting code would need to take the pointer size into account (one of the bugs I want to solve is that we're always assuming a 64-bit stack, whereas dwarf says it's address-sized). Knowing the address size at some point is unavoidable, so this might not be a big issue.
- it might interfere with subsequent optimizations (the scope of this is unknown to me)

Given that, it seems to me that it would be better (in line what @jmorse mentioned) to give the fragment information more prominence in the DIExpression, and enable emission code to better reason about it. That way it should be pretty easy to re-fragment the expression, if necessary, or just bail out (like now) in case we run into something we don't support.  Since DW_OP_LLVM_fragment only makes sense at the end of an expression (does it?), it's not clear to me if it's really useful to have it be a part of that expression in the first place, but it does not really matter, I think -- we could either keep them there, and create an abstraction that would let skip/ignore them, or try move them out completely.

What do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92013/new/

https://reviews.llvm.org/D92013