[PATCH] IR: Move the complex address expression field out of DIVariable and into an extra argument of the dbg.declare/dbg.value intrinsics.

[David Blaikie]

On Thu, Aug 21, 2014 at 5:54 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>! In D4919#11, @dblaikie wrote:
>>>>! In D4919#10, @aprantl wrote:
>>> If we also bump the metadata version number with this patch, the debug info will be stripped. Originally I thought that AsmParser would still throw an error because the intrinsics are missing an argument, but this is not the case, I just verified that. So this is good, that removes the autoupgrade requirement!
>>
>> Excellent!
>>
>>> My rationale for DW_TAG_user_lo is that this way we can guarantee that all DIDescriptor tags (except line table entries) are within the range [DW_TAG_array_type | LLVM_DebugVersion .. DW_TAG_user_hi|LLVM_DebugVersion].
>>
>> In what way is this beneficial? I suppose it helps us reserve different namespaces for different metadata annotations.
>
> Exactly!

But the thing is other metadata might not even start with ints - it
just happens that our pseudo-DWARF scheme did, so that's what we
picked up on. For some of these things we could probably be more
precise based on context and drop the tags (though if we did it'd be
harder to do the annotated comments, of course).

>> Are we annotating any other metadata? How are we identifying that metadata? If we are, I assume we'd want an overall scheme, but I doubt we are.
>
> Currently we aren't (in fact, TBAA is the *only* other kind of metadata that I'm aware of), but I think it would be nice if we could in the future.
> The -blocks testcases are a nice example of just how useful having a human-readable comment can be.

Oh, all the test cases are clear why the annotation comments are
useful (I tend to test against those rather than the metadata these
days, because that way the tests are schema-independent (ie: it
doesn't matter whether we add/remove a field, if it's the 3rd field or
the 4th field, etc)).

> If you feel that using the DW_TAG_user range has too much of a potential for conflicts, we could also allocate an adjacent range.

Looking at this further - I think it might make more sense to just
pick a special number (probably put it in Dwarf.h:LLVMConstants, and
make it DW_TAG_arg_variable + 1) as a prefix to the whole expression
list - rather than having to detect all possible DW_OPs that might
appear at the start of an expression.

>
>>
>>> My current suggestion is to use the end of the DW_TAG_user range for this purpose:
>>> ```
>>>     enum ComplexAddrKind {
>>>       OpFirst = dwarf::DW_TAG_hi_user - dwarf::DW_OP_hi_user,
>>>       OpPlus  = OpFirst + dwarf::DW_OP_plus,
>>>       OpDeref = OpFirst + dwarf::DW_OP_deref,
>>>       OpPiece = OpFirst + dwarf::DW_OP_piece,
>>>       OpLast = OpPiece
>>>     };
>>
>> The reason I shy away from this is that I think, as I mentioned, it confuses what these values are - they're not tags, they're just identifiers. (I mean I suppose the same is true of DW_TAG_arg_variable/param_variable or whatever it is we added that are non-standard 'tags' - but that's a bit more justified because they're in the same part of the schema - they have to be unique there/in the same range)
>>
>>>
>>> ```
>
> http://reviews.llvm.org/D4919
>
>