[PATCH] WIP: DI: Fold constant arguments into a single MDString

[David Blaikie]

On Tue, Sep 9, 2014 at 3:04 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> This patch addresses the first stage of PR17891 by folding constant
> arguments together into a single MDString.  Integers are stringified and
> a `\0` character is used as a separator.
>
> ----
>
> Known defects:
>
>   - I've only barely started to update testcases.
>   - There are bound to be bugs ~~~~~~~~~~~~^
>   - I deleted dead code (I'll separate that out before committing).
>
> I'd really appreciate some early feedback on this nevertheless.  I'm
> finding the testcases painful enough to update that I don't want to do it
> twice.
>
>  1. I think `\0` is cleaner than `,`.  Bike shed?
>

OK by me, but depending on how much schema and test case churn you're
inclined to (I know you folks at Apple are a bit more concerned about
schema churn than I am - because you have static bitcode archives, etc) we
might want to discuss the whole thing in a bit more detail. Things like:

* how's the scheme going to extend to hierarchical data? (eventually we'll
hopefully have rolled most of the whole DIE tree into single big strings -
separate out the types (which are still huge/non-trivial with all the
member function declarations, etc), etc)
* Do we want to consider a more human readable form (perhaps with an
internal clang flag to force it to produce that version of the metadata?)
for the purpose of writing test cases? Perhaps self-describing, so we can
more easily omit irrelevant fields from tests (when the file/line/col
doesn't matter for a particular declaration, etc).

If you're willing to churn this more later, I don't mind deferring these
decisions, if not - we might want to plan a bit more carefully.


>  2. I'm using hex for the tags.  Good idea?
>

Probably - that's the way they're written in the DWARF standard (&
hopefully in our Dwarf.h too).


>  3. I didn't add a prefix, which David suggested just now in PR17891.
>     This would be the time to do it, but is the hex tag sufficient?
>

The reason for the suggestion was to generalize the metadata annotation
scheme a bit, but we don't have anything to generalize over yet. If we're
willing to have the false positives of trying to render metadata
annotations on any other metadata that starts with a string starting with
"0x.." (we can/are a bit more precise than that - checking it has the right
number of fields, etc), etc... then no big deal.

Again, we're no worse off with what you've got here than what we have today
- and some time in the future we might want to add a consistent/more
general schema. *shrug*


>  4. Anything (other than testcases) I've missed updating?
>
>  5. Do you have specific advice on automating the testcase update flow?
>     My next step is building tool support, so suggestions (and starter
>     scripts) are welcome.
>
>
HeaderTwine doesn't look like a Twine - it looks like a string builder of
sorts. (I'd avoid using the term twine for something that doesn't do the
magic delayed string concatenation that llvm's twine does) maybe
HeaderString?

Some of the Verify implementations look off - old num operands == new num
operands + header fields - 1 (minus one for the header itself being counted
in the number of operands), right? Some don't add up
(DITemplateTypeParameter::Verify went from 7 to 4 + 3, so isn't it missing
a field?

Any size measurements/test results (does this bootstrap?) on this?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140915/50645e2d/attachment.html>