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

Manman Ren mren at apple.com
Fri Jun 21 22:01:57 PDT 2013


Some scoping for option a:

There are a few Verify functions
DISubprogram::Verify
DICompositeType::Verify
DIType::Verify
that try to access the context link:
  if (getContext() && !getContext().Verify())
    return false;

And the Verify functions are called from 10+ files.

If we are oaky with not calling getContext().Verify(), option a) appears much better to me.
For printing | debugging purpose, we agree that not being able to trace the pointer is okay.

Let me know your thoughts,
Manman

On Jun 21, 2013, at 5:01 PM, Eric Christopher 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.
> 
> -eric




More information about the llvm-dev mailing list