[PATCH] AsmWriter/Bitcode: Support specialized debug nodes

[Adrian Prantl]

> On Feb 10, 2015, at 6:29 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> 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:’.

yes, sure!
> 
>>> 
>>> 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?

Sufficient as an intermediate step on our way to the above expression being written as

  !MDExpression(DW_OP_deref, DW_OP_constu(8), DW_OP_plus, DW_OP_bit_piece(offset: 0, size: 8))

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