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

[Eric Christopher]

> I am referring to the fact nobody else had comments on our proposal of
> modifying MDNode :]
> That probably means we are the ones who care the most about MDNode?
>

Undoubtedly :)

-eric

> Thanks,
> Manman
>
>
>
> Thanks,
> Manman
>
> On Jun 21, 2013, at 10:01 PM, Manman Ren wrote:
>
>
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>