r247303 - Debug Info: Remove an unnecessary debug type visitor.

Thu Sep 10 10:38:45 PDT 2015

On Thu, Sep 10, 2015 at 10:25 AM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Sep 10, 2015, at 10:19 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Sep 10, 2015 at 10:18 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>>
>>
>> On Thu, Sep 10, 2015 at 10:13 AM, Adrian Prantl via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: adrian
>>> Date: Thu Sep 10 12:13:31 2015
>>> New Revision: 247303
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=247303&view=rev
>>> Log:
>>> Debug Info: Remove an unnecessary debug type visitor.
>>> Thanks to dblaikie for spotting this.
>>>
>>> Modified:
>>>     cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
>>>     cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>>>
>>> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303&r1=247302&r2=247303&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep
>>> 10 12:13:31 2015
>>> @@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST
>>>        return true;
>>>      }
>>>
>>> -    bool VisitValueDecl(ValueDecl *D) {
>>> -      QualType QualTy = D->getType();
>>> -      if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
>>> -        DI.getOrCreateStandaloneType(QualTy, D->getLocation());
>>> -      return true;
>>> -    }
>>> -
>>>      bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
>>>        QualType QualTy(D->getTypeForDecl(), 0);
>>>        if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
>>>
>>> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303&r1=247302&r2=247303&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
>>> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015
>>> @@ -7,10 +7,12 @@
>>>  // RUN: rm -rf %t
>>>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++
>>> -std=c++11 -g -fmodules -fmodule-format=obj -fimplicit-module-maps
>>> -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll
>>> -mllvm -debug-only=pchcontainer &>%t-mod.ll
>>>  // RUN: cat %t-mod.ll | FileCheck %s
>>> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>>>
>>>  // PCH:
>>>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11
>>> -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h
>>> -mllvm -debug-only=pchcontainer &>%t-pch.ll
>>>  // RUN: cat %t-pch.ll | FileCheck %s
>>> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>>>
>>>  #ifdef MODULES
>>>  @import DebugCXX;
>>> @@ -30,12 +32,11 @@
>>>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
>>>  // CHECK-SAME:             name: "Template<float,
>>> DebugCXX::traits<float> >"
>>>  // CHECK-SAME:             identifier:
>>> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
>>> -// CHECK: !DICompositeType(tag: DW_TAG_class_type,
>>> -// CHECK-SAME:             name: "Template<long, DebugCXX::traits<long>
>>> >"
>>> -// CHECK-SAME:             identifier:
>>> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")
>>>  // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
>>>  // CHECK-SAME:             identifier: "_ZTSN8DebugCXX1AIJvEEE")
>>>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
>>>  // no mangled name here yet.
>>>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
>>>  // no mangled name here yet.
>>> +
>>> +// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE"
>>>
>>
>> Rather than using a separate check - maybe do the same sort of thing we
>> do for DWARF testing, CHECK-NOT between each DICompositeType, to ensure we
>> only get the types we intended? (including a CHECK-NOT at the end to ensure
>> there aren't any trailing ones)
>>
>> But also: How does the current implementation avoid emitting this type? I
>> thought it visited all the type decls, even those not immediately in the
>> module? (you mentioned that was something that you were planning to address
>> in a future patch) Is that not the case? Is this now sufficient to only
>> emit the decls immediately in this module?
>>
>
> It transitively emits all types that are showing up in a type declaration
> in the module. This type is only instantiated inside a variable declaration.
>
> (Oh, I guess what might be missing is types referenced from /types/ in
> this module - types referenced from variable decls only are addressed by
> this patch, but you still don't want to include a full definition of
> std::vector just because a type derives from it, has a member of it, etc)
>
>
> Unless there is a typedef for that instantiation of std::vector in a
> module I don’t see a way of avoiding this currently.
>

Not sure I follow - you mean you're not sure how to detect that the type
(std::vector<int>, say, in this example) is defined within the module or
just referenced by it? Or even if you could detect that there's some other
difficult/limitation that makes it unavoidable?

>
> -- adrian
>
>
>
>>
>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150910/322d48e8/attachment-0001.html>