[PATCH] AsmWriter/Bitcode: Support specialized debug nodes

[Adrian Prantl]

> 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.
> 
> 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 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)

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. This doesn’t need to happen right away, but we should keep it in mind.

> 
> 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 :-)

-- adrian

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