r212263 - Move the calling of emitTargetMD() later.

Richard Smith richard at metafoo.co.uk
Tue Jul 15 20:07:24 PDT 2014


On Thu, Jul 3, 2014 at 2:30 AM, Robert Lytton <robert at xmos.com> wrote:

> Author: rlytton
> Date: Thu Jul  3 04:30:33 2014
> New Revision: 212263
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212263&view=rev
> Log:
> Move the calling of emitTargetMD() later.
>
> Summary:
> Because a global created by GetOrCreateLLVMGlobal() is not finalised until
> later viz:
>   extern char a[];
>   char f(){ return a[5];}
>   char a[10];
>
> Change MangledDeclNames to use a MapVector rather than a DenseMap so that
> the
> Metadata is output in order of original declaration, so to make
> deterministic
> and improve human readablity.
>

Sorry for not reviewing this before. I think this is completely the wrong
approach. Please just keep a list of those variables for which you couldn't
compute the type string when they are emitted (within
XCoreTargetCodeGenInfo), and perform another pass at the end of IR
generation to fix up those ones. That will have much better locality, and
will avoid the target metadata emission loop that is redundant on almost
all targets.

Differential Revision: http://reviews.llvm.org/D4176
>
> Modified:
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.h
>     cfe/trunk/test/CodeGen/xcore-stringtype.c
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=212263&r1=212262&r2=212263&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Jul  3 04:30:33 2014
> @@ -393,6 +393,8 @@ void CodeGenModule::Release() {
>      DebugInfo->finalize();
>
>    EmitVersionIdentMetadata();
> +
> +  EmitTargetMetadata();
>  }
>
>  void CodeGenModule::UpdateCompletedType(const TagDecl *TD) {
> @@ -1534,8 +1536,6 @@ CodeGenModule::GetOrCreateLLVMFunction(S
>      }
>    }
>
> -  getTargetCodeGenInfo().emitTargetMD(D, F, *this);
> -
>    // Make sure the result is of the requested type.
>    if (!IsIncompleteFunction) {
>      assert(F->getType()->getElementType() == Ty);
> @@ -1685,8 +1685,6 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
>    if (AddrSpace != Ty->getAddressSpace())
>      return llvm::ConstantExpr::getAddrSpaceCast(GV, Ty);
>
> -  getTargetCodeGenInfo().emitTargetMD(D, GV, *this);
> -
>    return GV;
>  }
>
> @@ -3340,6 +3338,14 @@ void CodeGenModule::EmitVersionIdentMeta
>    IdentMetadata->addOperand(llvm::MDNode::get(Ctx, IdentNode));
>  }
>
> +void CodeGenModule::EmitTargetMetadata() {
> +  for (auto &I : MangledDeclNames) {
> +    const Decl *D = I.first.getDecl()->getMostRecentDecl();
>

This has introduced an iterator invalidation bug that breaks most modules
builds: getMostRecentDecl can result in lazy deserialization, and thus in
MangledDeclNames having entries added to it.


> +    llvm::GlobalValue *GV = GetGlobalValue(I.second);
> +    getTargetCodeGenInfo().emitTargetMD(D, GV, *this);
> +  }
> +}
> +
>  void CodeGenModule::EmitCoverageFile() {
>    if (!getCodeGenOpts().CoverageFile.empty()) {
>      if (llvm::NamedMDNode *CUNode = TheModule.getNamedMetadata("
> llvm.dbg.cu")) {
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=212263&r1=212262&r2=212263&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Thu Jul  3 04:30:33 2014
> @@ -349,8 +349,8 @@ class CodeGenModule : public CodeGenType
>    /// emitted when the translation unit is complete.
>    CtorList GlobalDtors;
>
> -  /// A map of canonical GlobalDecls to their mangled names.
> -  llvm::DenseMap<GlobalDecl, StringRef> MangledDeclNames;
> +  /// An ordered map of canonical GlobalDecls to their mangled names.
> +  llvm::MapVector<GlobalDecl, StringRef> MangledDeclNames;
>    llvm::StringMap<GlobalDecl, llvm::BumpPtrAllocator> Manglings;
>
>    /// Global annotations.
> @@ -1146,6 +1146,9 @@ private:
>    /// \brief Emit the Clang version as llvm.ident metadata.
>    void EmitVersionIdentMetadata();
>
> +  /// Emits target specific Metadata for global declarations.
> +  void EmitTargetMetadata();
> +
>    /// Emit the llvm.gcov metadata used to tell LLVM where to emit the
> .gcno and
>    /// .gcda files in a way that persists in .bc files.
>    void EmitCoverageFile();
>
> Modified: cfe/trunk/test/CodeGen/xcore-stringtype.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/xcore-stringtype.c?rev=212263&r1=212262&r2=212263&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGen/xcore-stringtype.c (original)
> +++ cfe/trunk/test/CodeGen/xcore-stringtype.c Thu Jul  3 04:30:33 2014
> @@ -9,13 +9,13 @@
>  // Please see 'Tools Development Guide' section 2.16.2 for format details:
>  // <
> https://www.xmos.com/download/public/Tools-Development-Guide%28X9114A%29.pdf
> >
>
> -// CHECK: !xcore.typestrings = !{!0, !1, !2, !3, !4, !5, !6, !7, !8, !9,
> !10,
> -// CHECK: !11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23,
> -// CHECK: !24, !25, !26, !27, !28, !29, !30, !31, !32, !33, !34}
> +// CHECK: !xcore.typestrings = !{!1, !2, !3, !4, !5, !6, !7, !8, !9, !10,
> !11,
> +// CHECK: !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23,
> !24, !25,
> +// CHECK: !26, !27, !28, !29, !30, !31, !32, !33, !34, !35, !36, !37, !38}
>

Please fix this test to not hardcode metadata numbers; these are extremely
fragile, and will break if we start emitting more metadata for any reason.
Instead, use FileCheck variables (
http://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-variables).


>  // test BuiltinType
> -// CHECK: !0 = metadata !{void (i1, i8, i8, i8, i16, i16, i16, i32, i32,
> i32,
> +// CHECK: !1 = metadata !{void (i1, i8, i8, i8, i16, i16, i16, i32, i32,
> i32,
>  // CHECK:      i32, i32, i32, i64, i64, i64, float, double, double)*
>  // CHECK:      @builtinType, metadata
> !"f{0}(b,uc,uc,sc,ss,us,ss,si,ui,si,sl,
>  // CHECK:      ul,sl,sll,ull,sll,ft,d,ld)"}
> @@ -28,14 +28,14 @@ double _Complex Complex; // not supporte
>
>
>  // test FunctionType & Qualifiers
> -// CHECK: !1 = metadata !{void ()* @gI, metadata !"f{0}()"}
> -// CHECK: !2 = metadata !{void (...)* @eI, metadata !"f{0}()"}
> -// CHECK: !3 = metadata !{void ()* @gV, metadata !"f{0}(0)"}
> -// CHECK: !4 = metadata !{void ()* @eV, metadata !"f{0}(0)"}
> -// CHECK: !5 = metadata !{void (i32, ...)* @gVA, metadata !"f{0}(si,va)"}
> -// CHECK: !6 = metadata !{void (i32, ...)* @eVA, metadata !"f{0}(si,va)"}
> -// CHECK: !7 = metadata !{i32* (i32*)* @gQ, metadata
> !"f{crv:p(cv:si)}(p(cv:si))"}
> -// CHECK: !8 = metadata !{i32* (i32*)* @eQ, metadata
> !"f{crv:p(cv:si)}(p(cv:si))"}
> +// CHECK: !2 = metadata !{void ()* @gI, metadata !"f{0}()"}
> +// CHECK: !3 = metadata !{void (...)* @eI, metadata !"f{0}()"}
> +// CHECK: !4 = metadata !{void ()* @gV, metadata !"f{0}(0)"}
> +// CHECK: !5 = metadata !{void ()* @eV, metadata !"f{0}(0)"}
> +// CHECK: !6 = metadata !{void (i32, ...)* @gVA, metadata !"f{0}(si,va)"}
> +// CHECK: !7 = metadata !{void (i32, ...)* @eVA, metadata !"f{0}(si,va)"}
> +// CHECK: !8 = metadata !{i32* (i32*)* @gQ, metadata
> !"f{crv:p(cv:si)}(p(cv:si))"}
> +// CHECK: !9 = metadata !{i32* (i32*)* @eQ, metadata
> !"f{crv:p(cv:si)}(p(cv:si))"}
>  extern void eI();
>  void gI() {eI();};
>  extern void eV(void);
> @@ -49,10 +49,10 @@ const volatile int* volatile restrict co
>
>
>  // test PointerType
> -// CHECK: !9 = metadata !{i32* (i32*, i32* (i32*)*)*
> -// CHECK:      @pointerType, metadata
> !"f{p(si)}(p(si),p(f{p(si)}(p(si))))"}
> -// CHECK: !10 = metadata !{i32** @EP, metadata !"p(si)"}
> -// CHECK: !11 = metadata !{i32** @GP, metadata !"p(si)"}
> +// CHECK: !10 = metadata !{i32* (i32*, i32* (i32*)*)*
> +// CHECK:       @pointerType, metadata
> !"f{p(si)}(p(si),p(f{p(si)}(p(si))))"}
> +// CHECK: !11 = metadata !{i32** @EP, metadata !"p(si)"}
> +// CHECK: !12 = metadata !{i32** @GP, metadata !"p(si)"}
>  extern int* EP;
>  int* GP;
>  int* pointerType(int *I, int * (*FP)(int *)) {
> @@ -60,25 +60,25 @@ int* pointerType(int *I, int * (*FP)(int
>  }
>
>  // test ArrayType
> -// CHECK: !12 = metadata !{[2 x i32]* (i32*, i32*, [2 x i32]*, [2 x
> i32]*, i32*)*
> +// CHECK: !13 = metadata !{[2 x i32]* (i32*, i32*, [2 x i32]*, [2 x
> i32]*, i32*)*
>  // CHECK:       @arrayType, metadata
> !"f{p(a(2:si))}(p(si),p(cv:si),p(a(2:si)),
>  // CHECK:       p(a(2:si)),p(si))"}
> -// CHECK: !13 = metadata !{[0 x i32]* @EA1, metadata !"a(*:cv:si)"}
> -// CHECK: !14 = metadata !{[2 x i32]* @EA2, metadata !"a(2:si)"}
> -// CHECK: !15 = metadata !{[0 x [2 x i32]]* @EA3, metadata
> !"a(*:a(2:si))"}
> -// CHECK: !16 = metadata !{[3 x [2 x i32]]* @EA4, metadata
> !"a(3:a(2:si))"}
> -// CHECK: !17 = metadata !{[2 x i32]* @GA1, metadata !"a(2:cv:si)"}
> -// CHECK: !18 = metadata !{void ([2 x i32]*)* @arrayTypeVariable1,
> +// CHECK: !14 = metadata !{[0 x i32]* @EA1, metadata !"a(*:cv:si)"}
> +// CHECK: !15 = metadata !{[2 x i32]* @EA2, metadata !"a(2:si)"}
> +// CHECK: !16 = metadata !{[0 x [2 x i32]]* @EA3, metadata
> !"a(*:a(2:si))"}
> +// CHECK: !17 = metadata !{[3 x [2 x i32]]* @EA4, metadata
> !"a(3:a(2:si))"}
> +// CHECK: !18 = metadata !{[2 x i32]* @GA1, metadata !"a(2:cv:si)"}
> +// CHECK: !19 = metadata !{void ([2 x i32]*)* @arrayTypeVariable1,
>  // CHECK:       metadata !"f{0}(p(a(2:si)))"}
> -// CHECK: !19 = metadata !{void (void ([2 x i32]*)*)* @arrayTypeVariable2,
> +// CHECK: !20 = metadata !{void (void ([2 x i32]*)*)* @arrayTypeVariable2,
>  // CHECK:       metadata !"f{0}(p(f{0}(p(a(2:si)))))"}
> -// CHECK: !20 = metadata !{[3 x [2 x i32]]* @GA2, metadata
> !"a(3:a(2:si))"}
> +// CHECK: !21 = metadata !{[3 x [2 x i32]]* @GA2, metadata
> !"a(3:a(2:si))"}
> +extern int GA2[3][2];
>  extern const volatile int EA1[];
>  extern int EA2[2];
>  extern int EA3[][2];
>  extern int EA4[3][2];
>  const volatile int GA1[2];
> -int GA2[3][2];
>  extern void arrayTypeVariable1(int[*][2]);
>  extern void arrayTypeVariable2( void(*fp)(int[*][2]) );
>  extern void arrayTypeVariable3(int[3][*]);                // not supported
> @@ -100,16 +100,16 @@ RetType* arrayType(int A1[], int const v
>
>
>  // test StructureType
> -// CHECK: !21 = metadata !{void (%struct.S1*)* @structureType1, metadata
> +// CHECK: !22 = metadata !{void (%struct.S1*)* @structureType1, metadata
>  // CHECK:
> !"f{0}(s(S1){m(ps2){p(s(S2){m(ps3){p(s(S3){m(s1){s(S1){}}})}})}})"}
> -// CHECK: !22 = metadata !{void (%struct.S2*)* @structureType2, metadata
> +// CHECK: !23 = metadata !{void (%struct.S2*)* @structureType2, metadata
>  // CHECK:
> !"f{0}(s(S2){m(ps3){p(s(S3){m(s1){s(S1){m(ps2){p(s(S2){})}}}})}})"}
> -// CHECK: !23 = metadata !{void (%struct.S3*)* @structureType3, metadata
> +// CHECK: !24 = metadata !{void (%struct.S3*)* @structureType3, metadata
>  // CHECK:
> !"f{0}(s(S3){m(s1){s(S1){m(ps2){p(s(S2){m(ps3){p(s(S3){})}})}}}})"}
> -// CHECK: !24 = metadata !{void (%struct.S4*)* @structureType4, metadata
> +// CHECK: !25 = metadata !{void (%struct.S4*)* @structureType4, metadata
>  // CHECK:
> !"f{0}(s(S4){m(s1){s(S1){m(ps2){p(s(S2){m(ps3){p(s(S3){m(s1){s(S1){}}})}})}}}})"}
> -// CHECK: !25 = metadata !{%struct.anon* @StructAnon, metadata
> !"s(){m(A){si}}"}
> -// CHECK: !26 = metadata !{i32 (%struct.SB*)* @structureTypeB, metadata
> +// CHECK: !26 = metadata !{%struct.anon* @StructAnon, metadata
> !"s(){m(A){si}}"}
> +// CHECK: !27 = metadata !{i32 (%struct.SB*)* @structureTypeB, metadata
>  // CHECK:       !"f{si}(s(SB){m(){b(4:si)},m(){b(2:si)},m(N4){b(4:si)},
>  // CHECK:       m(N2){b(2:si)},m(){b(4:ui)},m(){b(4:si)},m(){b(4:c:si)},
>  // CHECK:       m(){b(4:c:si)},m(){b(4:cv:si)}})"}
> @@ -130,16 +130,16 @@ int structureTypeB(struct SB sb){return
>
>
>  // test UnionType
> -// CHECK: !27 = metadata !{void (%union.U1*)* @unionType1, metadata
> +// CHECK: !28 = metadata !{void (%union.U1*)* @unionType1, metadata
>  // CHECK:
> !"f{0}(u(U1){m(pu2){p(u(U2){m(pu3){p(u(U3){m(u1){u(U1){}}})}})}})"}
> -// CHECK: !28 = metadata !{void (%union.U2*)* @unionType2, metadata
> +// CHECK: !29 = metadata !{void (%union.U2*)* @unionType2, metadata
>  // CHECK:
> !"f{0}(u(U2){m(pu3){p(u(U3){m(u1){u(U1){m(pu2){p(u(U2){})}}}})}})"}
> -// CHECK: !29 = metadata !{void (%union.U3*)* @unionType3, metadata
> +// CHECK: !30 = metadata !{void (%union.U3*)* @unionType3, metadata
>  // CHECK:
> !"f{0}(u(U3){m(u1){u(U1){m(pu2){p(u(U2){m(pu3){p(u(U3){})}})}}}})"}
> -// CHECK: !30 = metadata !{void (%union.U4*)* @unionType4, metadata
> +// CHECK: !31 = metadata !{void (%union.U4*)* @unionType4, metadata
>  // CHECK:
> !"f{0}(u(U4){m(u1){u(U1){m(pu2){p(u(U2){m(pu3){p(u(U3){m(u1){u(U1){}}})}})}}}})"}
> -// CHECK: !31 = metadata !{%union.anon* @UnionAnon, metadata
> !"u(){m(A){si}}"}
> -// CHECK: !32 = metadata !{i32 (%union.UB*)* @unionTypeB, metadata
> +// CHECK: !32 = metadata !{%union.anon* @UnionAnon, metadata
> !"u(){m(A){si}}"}
> +// CHECK: !33 = metadata !{i32 (%union.UB*)* @unionTypeB, metadata
>  // CHECK:       !"f{si}(u(UB){m(N2){b(2:si)},m(N4){b(4:si)},m(){b(2:si)},
>  // CHECK:
> m(){b(4:c:si)},m(){b(4:c:si)},m(){b(4:cv:si)},m(){b(4:si)},
>  // CHECK:       m(){b(4:si)},m(){b(4:ui)}})"}
> @@ -160,9 +160,20 @@ int unionTypeB(union UB ub) {return Unio
>
>
>  // test EnumType
> -// CHECK: !33 = metadata !{i32* @EnumAnon, metadata !"e(){m(EA){3}}"}
> -// CHECK: !34 = metadata !{i32 (i32)* @enumType, metadata
> +// CHECK: !34 = metadata !{i32* @EnumAnon, metadata !"e(){m(EA){3}}"}
> +// CHECK: !35 = metadata !{i32 (i32)* @enumType, metadata
>  // CHECK:       !"f{si}(e(E){m(A){7},m(B){6},m(C){5},m(D){0}})"}
>  enum E {D, C=5, B, A};
>  enum {EA=3} EnumAnon = EA;
>  int enumType(enum E e) {return EnumAnon;}
> +
> +
> +// CHECK: !36 = metadata !{i32 ()* @testReDecl, metadata !"f{si}()"}
> +// CHECK: !37 = metadata !{[10 x i32]* @After, metadata !"a(10:si)"}
> +// CHECK: !38 = metadata !{[10 x i32]* @Before, metadata !"a(10:si)"}
> +extern int After[];
> +extern int Before[10];
> +int testReDecl() {return After[0] + Before[0];}
> +int After[10];
> +int Before[];
> +
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140715/c3ee3b41/attachment.html>


More information about the cfe-commits mailing list