[PATCH] D43199: [IRMover] Implement name based structure type mapping

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 20 16:05:30 PST 2018


Thanks. I am doing a few experiments and should report by the end of the
day.

Cheers,
Rafael

Eugene Leviant via Phabricator <reviews at reviews.llvm.org> writes:

> evgeny777 updated this revision to Diff 134444.
> evgeny777 added a comment.
>
> Addressed review comments in the mailing list:
>
> - added --initial-module flag to llvm-link
> - test cases now use llvm-link instead of llvm-lto
>
>
> https://reviews.llvm.org/D43199
>
> Files:
>   include/llvm/Linker/IRMover.h
>   lib/Linker/IRMover.cpp
>   test/Linker/Inputs/struct-mapping-baz.ll
>   test/Linker/Inputs/struct-mapping-foo.ll
>   test/Linker/struct-mapping.ll
>   tools/llvm-link/llvm-link.cpp
>
> Index: tools/llvm-link/llvm-link.cpp
> ===================================================================
> --- tools/llvm-link/llvm-link.cpp
> +++ tools/llvm-link/llvm-link.cpp
> @@ -70,6 +70,11 @@
>  OutputFilename("o", cl::desc("Override output filename"), cl::init("-"),
>                 cl::value_desc("filename"));
>  
> +static cl::opt<std::string>
> +InitialModule("initial-module",
> +              cl::desc("Link to existing destination module"), cl::init(""),
> +              cl::value_desc("filename"));
> +
>  static cl::opt<bool>
>  Internalize("internalize", cl::desc("Internalize linked symbols"));
>  
> @@ -360,7 +365,9 @@
>    if (!DisableDITypeMap)
>      Context.enableDebugTypeODRUniquing();
>  
> -  auto Composite = make_unique<Module>("llvm-link", Context);
> +  auto Composite = InitialModule.empty()
> +                       ? make_unique<Module>("llvm-link", Context)
> +                       : loadFile(argv[0], InitialModule, Context);
>    Linker L(*Composite);
>  
>    unsigned Flags = Linker::Flags::None;
> Index: test/Linker/struct-mapping.ll
> ===================================================================
> --- test/Linker/struct-mapping.ll
> +++ test/Linker/struct-mapping.ll
> @@ -0,0 +1,38 @@
> +; RUN: llvm-link --initial-module=%s %p/Inputs/struct-mapping-baz.ll %p/Inputs/struct-mapping-foo.ll -S -o - | FileCheck %s
> +
> +; Here we check that new type mapping algorithm correctly mapped type of internal
> +; member of struct.Baz to struct.Foo. Without it we'd map that type to struct.Bar, because
> +; it is recursively isomorphic to struct.Foo and is defined first in source file.
> +; CHECK: %struct.Baz = type { i64, i64, %struct.Foo } 
> +
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-pc-linux-gnu"
> +
> +%struct.Bar = type { i64, i64 }
> +%struct.S = type { %struct.Foo* }
> +%struct.Foo = type { i64, i64 }
> +
> + at B0 = local_unnamed_addr global %struct.Bar zeroinitializer, align 8
> + at F0 = local_unnamed_addr global %struct.Foo zeroinitializer, align 8
> + at instance = local_unnamed_addr global %struct.S zeroinitializer, align 8
> + at llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_main.cpp, i8* null }]
> +
> +; Function Attrs: norecurse nounwind readonly uwtable
> +define i32 @main() local_unnamed_addr {
> +entry:
> +  %0 = load %struct.Foo*, %struct.Foo** getelementptr inbounds (%struct.S, %struct.S* @instance, i64 0, i32 0), align 8
> +  %a = getelementptr inbounds %struct.Foo, %struct.Foo* %0, i64 0, i32 0
> +  %1 = load i64, i64* %a, align 8
> +  %conv = trunc i64 %1 to i32
> +  ret i32 %conv
> +}
> +
> +declare %struct.Foo* @_Z6getFoov() local_unnamed_addr
> +
> +; Function Attrs: uwtable
> +define internal void @_GLOBAL__sub_I_main.cpp() section ".text.startup" {
> +entry:
> +  %call.i.i = tail call %struct.Foo* @_Z6getFoov()
> +  store %struct.Foo* %call.i.i, %struct.Foo** getelementptr inbounds (%struct.S, %struct.S* @instance, i64 0, i32 0), align 8
> +  ret void
> +}
> Index: test/Linker/Inputs/struct-mapping-foo.ll
> ===================================================================
> --- test/Linker/Inputs/struct-mapping-foo.ll
> +++ test/Linker/Inputs/struct-mapping-foo.ll
> @@ -0,0 +1,26 @@
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-pc-linux-gnu"
> +
> +%struct.Foo = type { i64, i64 }
> +%struct.Baz = type { i64, i64, %struct.Foo }
> +
> + at gFoo = local_unnamed_addr global %struct.Foo* null, align 8
> +
> +; Function Attrs: uwtable
> +define %struct.Foo* @_Z6getFoov() local_unnamed_addr {
> +entry:
> +  %0 = load %struct.Foo*, %struct.Foo** @gFoo, align 8
> +  %tobool = icmp eq %struct.Foo* %0, null
> +  br i1 %tobool, label %cond.false, label %cond.end
> +
> +cond.false:                                       ; preds = %entry
> +  %call = tail call %struct.Baz* @_Z6getBazv()
> +  %f = getelementptr inbounds %struct.Baz, %struct.Baz* %call, i64 0, i32 2
> +  br label %cond.end
> +
> +cond.end:                                         ; preds = %entry, %cond.false
> +  %cond = phi %struct.Foo* [ %f, %cond.false ], [ %0, %entry ]
> +  ret %struct.Foo* %cond
> +}
> +
> +declare %struct.Baz* @_Z6getBazv() local_unnamed_addr
> Index: test/Linker/Inputs/struct-mapping-baz.ll
> ===================================================================
> --- test/Linker/Inputs/struct-mapping-baz.ll
> +++ test/Linker/Inputs/struct-mapping-baz.ll
> @@ -0,0 +1,14 @@
> +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
> +target triple = "x86_64-pc-linux-gnu"
> +
> +%struct.Baz = type { i64, i64, %struct.Foo }
> +%struct.Foo = type { i64, i64 }
> +
> + at gBaz = local_unnamed_addr global %struct.Baz* null, align 8
> +
> +; Function Attrs: norecurse nounwind readonly uwtable
> +define %struct.Baz* @_Z6getBazv() local_unnamed_addr {
> +entry:
> +  %0 = load %struct.Baz*, %struct.Baz** @gBaz, align 8
> +  ret %struct.Baz* %0
> +}
> Index: lib/Linker/IRMover.cpp
> ===================================================================
> --- lib/Linker/IRMover.cpp
> +++ lib/Linker/IRMover.cpp
> @@ -319,7 +319,7 @@
>      }
>  
>      if (StructType *OldT =
> -            DstStructTypesSet.findNonOpaque(ElementTypes, IsPacked)) {
> +            DstStructTypesSet.findNonOpaque(ElementTypes, IsPacked, STy->getName())) {
>        STy->setName("");
>        return *Entry = OldT;
>      }
> @@ -906,7 +906,6 @@
>  Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
>                                                      bool ForAlias) {
>    GlobalValue *DGV = getLinkedToGlobal(SGV);
> -
>    bool ShouldLink = shouldLink(DGV, *SGV);
>  
>    // just missing from map
> @@ -1414,12 +1413,14 @@
>  
>  void IRMover::IdentifiedStructTypeSet::addNonOpaque(StructType *Ty) {
>    assert(!Ty->isOpaque());
> +  if (Ty->hasName())
> +    NonOpaqueStructNameMap.insert({getTypeNamePrefix(Ty->getName()), Ty});
> +
>    NonOpaqueStructTypes.insert(Ty);
>  }
>  
>  void IRMover::IdentifiedStructTypeSet::switchToNonOpaque(StructType *Ty) {
> -  assert(!Ty->isOpaque());
> -  NonOpaqueStructTypes.insert(Ty);
> +  addNonOpaque(Ty);
>    bool Removed = OpaqueStructTypes.erase(Ty);
>    (void)Removed;
>    assert(Removed);
> @@ -1432,10 +1433,16 @@
>  
>  StructType *
>  IRMover::IdentifiedStructTypeSet::findNonOpaque(ArrayRef<Type *> ETypes,
> -                                                bool IsPacked) {
> +                                                bool IsPacked, StringRef Name) {
>    IRMover::StructTypeKeyInfo::KeyTy Key(ETypes, IsPacked);
>    auto I = NonOpaqueStructTypes.find_as(Key);
> -  return I == NonOpaqueStructTypes.end() ? nullptr : *I;
> +  if (I == NonOpaqueStructTypes.end())
> +    return nullptr;
> +  auto NI = NonOpaqueStructNameMap.find(getTypeNamePrefix(Name));
> +  if (NI != NonOpaqueStructNameMap.end() &&
> +      IRMover::StructTypeKeyInfo::KeyTy((*NI).second) == Key)
> +    return (*NI).second;
> +  return *I;
>  }
>  
>  bool IRMover::IdentifiedStructTypeSet::hasType(StructType *Ty) {
> Index: include/llvm/Linker/IRMover.h
> ===================================================================
> --- include/llvm/Linker/IRMover.h
> +++ include/llvm/Linker/IRMover.h
> @@ -52,11 +52,16 @@
>      // The set of identified but non opaque structures in the composite module.
>      DenseSet<StructType *, StructTypeKeyInfo> NonOpaqueStructTypes;
>  
> +    // Map between structure type name and instance. Used in findNonOpaque
> +    // to correctly map imported global variable type during ThinLTO import
> +    // phase.
> +    DenseMap<StringRef, StructType *> NonOpaqueStructNameMap;
>    public:
>      void addNonOpaque(StructType *Ty);
>      void switchToNonOpaque(StructType *Ty);
>      void addOpaque(StructType *Ty);
> -    StructType *findNonOpaque(ArrayRef<Type *> ETypes, bool IsPacked);
> +    StructType *findNonOpaque(ArrayRef<Type *> ETypes, bool IsPacked,
> +                              StringRef Name);
>      bool hasType(StructType *Ty);
>    };
>  


More information about the llvm-commits mailing list