[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