[PATCH] AsmWriter/Bitcode: Support specialized debug nodes

[Duncan P. N. Exon Smith]

> On 2015-Feb-10, at 16:59, Adrian Prantl <aprantl at apple.com> wrote:
> 
> 
>> On Feb 10, 2015, at 4:48 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> 
>>> On 2015-Feb-10, at 16:05, Adrian Prantl <aprantl at apple.com> wrote:
>>> 
>>> A few nitpicky suggestions and questions:
>>> 
>>> - !MDSubrange(count: 30, lo: 2)
>>> what about spelling out “low” and “high”?
>> 
>> It doesn't mean "low" and "high", though, right?  It means
>> "count"/"size" and "index of first element”.
> 
> You are probably right. I was thinking of DWARF where a subrange may either have a
>  DW_AT_lower_bound and/or a DW_AT_upper_bound
> or a
>  DW_AT_lower_bound and/or a DW_AT_count
> 
> We only support the second form so count and lower_bound would probably be most accurate.

Sounds good.  In keeping with the field naming convention I've adopted,
I'll use 'count:' and 'lowerBound:'.

>> 
>> Or have I misinterpreted it?
>> 
>> How about "count" and "low"?
>> 
>>> 
>>> - do we still need the uniqueID on MDLexicalBlock? Could it be a distinct node instead?
>> 
>> I think this should be distinct instead.  I was planning to do this
>> as part of the upgrade (moving them into place).
>> 
>>> 
>>> - is it possible to have it print DW_OP-* constants such as in
>>> !MDExpression(DW_OP_deref, DW_OP_bit_piece, 0, 8)
>>> ?
>> 
>> I think this is pretty difficult the way that expressions are stored
>> right now -- it requires knowing how many arguments each thing takes,
>> etc.  In particular, how would the `AsmWriter` know whether something
>> should have a symbolic constant (`DW_OP_deref`) vs. being the number 6?
> 
> It would have to reuse the code from DIExpression::printInternal().

I think this is doable.

On the LLParser side, I've generally allowed raw ints (for `DW_TAG*`,
`DW_LANG*`, etc.), but for `DW_OP`s we could (at least for now) require
symbolic constants.

>> 
>> I was thinking of trying to clean this up later, but since we're
>> talking about it:
>> 
>> I think instead of `std::vector<uint64_t>` it should be
>> `std::vector<ExprOperand>`, where:
>> 
>>   struct ExprOperand {
>>     LocationAtom Kind;
>>     unsigned NumArgs;
>>     uint64_t Args[2];
>>     ExprOperand(LocationAtom Kind);
>>     ExprOperand(LocationAtom Kind, uint64_t Arg);
>>     ExprOperand(LocationAtom Kind, uint64_t Arg1, uint64_t Arg2);
>> 
>>     static ExprOperand getDeref() {
>>       return ExprOperand(dwarf::DW_OP_deref);
>>     }
>>     static ExprOperand getPiece(uint64_t A1, uint64_t A2) {
>>       return ExprOperand(dwarf::DW_OP_bit_piece, A1, A2);
>>     }
>>   };
>> 
>>   ExprOperand Ops = {
>>     ExprOperand::getDeref(),
>>     ExprOperand::getPiece(0, 8)
>>   };
>>   auto *Expr = MDExpression::get(Ops);
>> 
>> Then the above could trivially be pretty-printed/parsed as:
>> 
>>   !MDExpression(DW_OP_deref, DW_OP_bit_piece(0, 8))
> 
> or even better ... DW_OP_bit_piece(offset: 0, size: 8)

Sure.

> Yeah that looks good. Also we would need to introduce DW_OP_constu and get rid of the pseudo argument that DW_OP_plus currently takes.

Not necessary.  You can represent this already with:

    DW_OP_plus(7)

where `7` is the pseudo-argument.  This is somewhat horrible, but
so is `DW_OP_plus` taking a pseudo-argument ;).

> This doesn’t need to happen right away, but we should keep it in mind.

I think something like this would simplify the logic a lot, besides
making it more extensible.

>> 
>> Thoughts?
>> 
>> 
>>> 
>>> Will it still print the old-style comments that testcases tend to match on, or are they obsoleted by the human-readable syntax?
>> 
>> They're obsoleted.  The old-style comments will get stale and just
>> add noise.  If we've regressed somehow let me know how.
> 
> See my comment on DW_OP_bit_piece above :-)
> 

Hah, right, right :).

I'll see if I can get something together for you.  If not we can keep
the DW_TAG_expression comments temporarily.

Is it alright to lose the "offset=" and "size=" markers (at least
temporarily)?  I don't see a natural place to put them in the syntax.
E.g., I was thinking I'd aim for the following for now:

    !MDExpression(DW_OP_deref, DW_OP_plus, 8, DW_OP_bit_piece, 0, 8)

(and with no comment).  Is this sufficient?

> 
>> 
>>> 
>>> -- adrian
>>> 
>>> 
>>> 
>> 
>