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

Adrian Prantl aprantl at apple.com
Tue Sep 23 09:52:25 PDT 2014


> 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? DIHeaderIterator derefs to a StringRef instead of a DIHeader object, which appears counterintuitive to me. 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?
All the other DI* classes are wrappers around MDNodes (and, more importantly, provide convenient accessors to the fields) and DIHeader breaks with that convention.

-- adrian



More information about the llvm-commits mailing list