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

[Duncan P. N. Exon Smith]

> On 2014-Sep-23, at 09:52, Adrian Prantl <aprantl at apple.com> wrote:
> 
>> 
>> On Sep 22, 2014, at 5:37 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> 
>>> On 2014-Sep-19, at 17:29, 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.
>>> 
>>> I've attached updated LLVM and clang patches, rebased this afternoon
>>> with all tests in `check-all` passing.  I believe it addresses David's
>>> comments as well.
>>> 
>>> There are an insane number of changes in the testcases, but changing the
>>> schema of the dwarf tags makes that inevitable -- almost every line of
>>> metadata had to change.  I look forward to "stage 2" of PR17891, which
>>> can be more incremental.
>>> 
>>> Let me know if the updated patches look good.
>>> 
>>> Two caveats that came to mind while writing this:
>>> 
>>> - I haven't updated `docs/SourceLevelDebugging.rst`.  I'll send a
>>>  patch for that Monday-ish.
>> 
>> Included in the attached LLVM patch.
>> 
>>>> Any size measurements/test results (does this bootstrap?) on this?
>>> 
>>> Before committing, I'll use a release+asserts clang to build:
>>> 
>>> - opt with `-g -flto`
>> 
>> I profiled `ld64`'s link of `llvm-lto`:
>> 
>> - baseline: 8m21s, 16.41 GB
>> - modified: 7m56s, 14.25 GB
>> 
>> The memory change is about what I expected.
> 
> Very nice!
> 
>> 
>>> - clang with `-g`
>> 
>> Bootstrap was clean.
>> 
>> --
>> 
>> Rebased patches attached.
>> 
>> <0001-DI-Header-MDString-llvm-v2.patch><0002-DI-Header-MDString-clang-v2.patch>
> 
> Because of the logistics involved in updating all the testcases, it would be best to commit this after http://reviews.llvm.org/D4919.
> 
> I have one question about the API design: Is DIHeader used for anything besides constructing MDStrings?

Nope.

> DIHeaderIterator derefs to a StringRef instead of a DIHeader object, which appears counterintuitive to me.

Renamed to `DIHeaderFieldIterator`, and renamed the unit test macro
to `MAKE_FIELD_ITERATOR`.

> I wonder if it would be feasible/sensible to turn DIHeader into a single variadic function template rather than a class. Or maybe DIHeader should be called something akin to DIHeaderBuilder?

It's a local class, so it doesn't need the `DI` prefix either.
Renamed it to `HeaderBuilder`.

> All the other DI* classes are wrappers around MDNodes (and, more importantly, provide convenient accessors to the fields) and DIHeader breaks with that convention.

Right, it was more misleading than I realized.

New patches attached.  (Clang patch is unchanged.)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-DI-Header-MDString-llvm-v3.patch
Type: application/octet-stream
Size: 2147899 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140923/c67e0ed4/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-DI-Header-MDString-clang-v3.patch
Type: application/octet-stream
Size: 93100 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140923/c67e0ed4/attachment-0001.obj>