[Type Uniquing Patch] Modify functions in DebugInfo.h to take an extra map argument.
dblaikie at gmail.com
Fri Jul 19 15:22:42 PDT 2013
On Fri, Jul 19, 2013 at 12:54 PM, Manman Ren <manman.ren at gmail.com> wrote:
> On Fri, Jul 19, 2013 at 11:14 AM, Manman Ren <mren at apple.com> wrote:
>> On Jul 18, 2013, at 9:58 PM, David Blaikie wrote:
>> > > About the Verify functions, the patch modifies from Verify() to
>> > > Verify(DIMap
>> > > &). Is this interface change okay?
>> > Again, I was hoping not to change the signature of these functions if
>> > possible.
>> Agreed. Let's drop anything from verify that needs type resolution, unless
>> there's a good reason not to. We should move verification to a single pass
>> at some point which could use resolution there,
>> To verify that a type operand is indeed a DIType, we have to perform type
>> resolution, similarly to verify that a context is indeed a context, we have
>> to perform type lookup.
>> We can verify each debug info node without performing lookup, but we will
>> not verify the links across nodes.
> If we rely on DebugInfoFinder to list all the context (DIScope), and then
> call Verify on those scopes, we can remove the check that getContext is
> indeed a DIScope from DIType::Verify.
Yes, this is the intent - all consistency checks should happen in the
Verifier, then the rest of LLVM can just assume that it's correct.
> Currently DebugInfoFinder already lists the derived types of a
> DIDerivedType, so there is no need to perform lookup and verify that
> getTypeDerivedFrom is indeed a DIType in DIDerivedType::Verify.
& sooner or later there should be no need to call
DIDerivedType::Verify from anywhere (other than the Verifier, if that
turns out to be the right/best way to write the Verifier).
> With the above, there is no need to pass Map to Verify functions. Let me if
> it sounds okay.
I think what you're saying is the same thing as what I'm saying...
More information about the llvm-commits