[PATCH] DebugInfo: Remove 'inlinedAt:' field from MDLocalVariable/DIVariable

[Duncan P. N. Exon Smith]

> On 2015-Apr-03, at 12:25, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
>> 
>> On 2015-Apr-02, at 09:14, David Blaikie <dblaikie at gmail.com> wrote:
>> 
>> 
>> 
>> On Thu, Apr 2, 2015 at 9:10 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>>> On 2015-Apr-02, at 08:01, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> 
>>> On Apr 1, 2015 11:36 PM, "Duncan P. N. Exon Smith" <dexonsmith at apple.com> wrote:
>>>> 
>>>> When I was gathering LTO heap profiles a few weeks ago, the single
>>>> largest `Metadata` contributer to memory usage was `MDLocalVariable`,
>>>> with a huge chunk coming from `MDLocalVariable::withInline()`.
>>>> 
>>>> This patch stops storing where an `MDLocalVariable` has been inlined.
>>>> This inlined-at info was used two ways:
>>>> 
>>>>  - To tell the backend where a variable was inlined.
>>>>  - To create a unique id for each inlined variable.
>>>> 
>>>> However, every instruction has a `!dbg` attachment with the same
>>>> information in its inlined-at field.  AFAICT, David Blaikie's work to
>>>> make that accurate has paid off, since this naive patch (which just
>>>> removes the field entirely) passes all the tests.
>>>> 
>>>> The unique id for inlined variables is now a typedef called
>>>> `InlinedVariable` of `std::pair<MDLocalVariable*, MDLocation*>`.
>>>> 
>>>> Besides the patch, I've attached the upgrade script I used to update
>>>> testcases.
>>>> 
>>>> Any reason not to commit?  (Am I missing something here?)
>>> 
>>> Last time I tried this (not sure whether I got as far as commuting/referring it) either I hit those issues I have since fixed (maybe that's part of what me going on that kick) or some other issues...
>>> 
>>> In any case, would it be possible to run some tests that assert that these two locations are the same (possibly this assertion could take the place of the construction of these excessive/redundant/bloated inlined DIVariables - so we could gain the memory savings while still leaving the checking in just in case there are new/weird/interesting cases) for now?
>> 
>> I can't remove the field and assert about its value at the same time ;).
>> 
>> Sorry, what I meant (& perhaps it's still not possible) is to assert, at the point where the field would be initialized/the extra DIVariable created, instead just assert that the inlinedAt location we were about to use is the same as the dbgloc we would've used.
>> 
>> I'll start by adding the assertion, and after a while (a day? a week?)
>> if no bot complains I'll rip out the field.  (If bots do complain we can
>> fix bugs until they're happy.)
>> 
>> *nod* Sounds good - I'm not too fussed about waiting ages, but at least a good clang/opt bootstrap, etc. A week's probably not unreasonable.
> 
> FYI, committed the checks in r234021 and r234038.

And finished the job with r235040, r235041, r235042, r235048, and
r235050.  See PR22778 for the testcase upgrade scripts.