[LLVMdev] Proposal: type uniquing of debug info for LTO

[Manman Ren]

On Jun 21, 2013, at 5:01 PM, Eric Christopher <echristo at gmail.com> wrote:

> On Fri, Jun 21, 2013 at 4:59 PM, David Blaikie <dblaikie at gmail.com> wrote:
>> On Fri, Jun 21, 2013 at 4:50 PM, Manman Ren <mren at apple.com> wrote:
>>> 
>>> On Jun 21, 2013, at 4:41 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> The reason I want a flag is to avoid the need to update the existing testing
>>> cases while this is a work in progress.
>>> I believe migrating one field means updating the existing testing cases?
>>> 
>>> 
>>> It would, yes - and that's how you'd get coverage to know your change
>>> was stable. You'll have to update all the tests eventually anyway &
>>> doing so incrementally isn't substantially more expensive in my
>>> experience. Perhaps there's something unique to this migration that
>>> would make it so?
>>> 
>>> 
>>> I am going to update the existing testing cases locally to have the test
>>> coverage, but I don't want to check in the changes often.
>>> Once we turn the flag on by default and remove the other path, I will submit
>>> the changes on the testing cases.
>> 
>> I'd rather skip the flag & have the test coverage in the open along
>> with the changes. Having changes without testing is bad. I don't see a
>> benefit to you keeping the test coverage locally.
>> 
>>> Another point to have a flag is to check in the patches in steps: DIBuilder
>>> changes, changes related to the map, and DwarfDebug changes.
>>> Without the flag, when I migrate the first field, I have to make sure it
>>> works from frontend all the way to the backend.
>> 
>> True enough, but I don't think that's a great drawback.
>> 
>> Though technically, if you really wanted, you could start at the
>> backend & make the implementation conditional (check the type of the
>> field, if it's an MDNode - use that, if it's a string, do the lookup)
>> - implement that, then port DIBuilder, then remove the conditionality.
>> Though I don't like it a lot & I think the changes will be small
>> enough that frontend to backend will be still be understandable
>> patches.
> 
> All of this.

+2
BTW, What is considered as small enough?

Manman
> 
> -eric