[PATCH] D43199: [IRMover] Implement name based structure type mapping
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 13 07:00:36 PST 2018
Why do you want this?
The name of LLVM types should not be important. The cases that we
already have are a hack and I think the agreed direction is to have a
single pointer type. That would make the type graph a DAG which is
trivial to merge.
Cheers,
Rafael
Eugene Leviant via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:
> evgeny777 created this revision.
> evgeny777 added reviewers: mehdi_amini, tejohnson.
> Herald added a subscriber: eraman.
>
> This patch was separated from https://reviews.llvm.org/D43077 after discussion with @tejohnson. It implements name based structure type mapping to
> avoid mapping to wrong type which is isomorphic to one we're about to import.
>
> Not sure whom to add for review.
>
>
> https://reviews.llvm.org/D43199
>
> Files:
> include/llvm/Linker/IRMover.h
> lib/Linker/IRMover.cpp
> test/ThinLTO/X86/Inputs/struct-mapping-baz.ll
> test/ThinLTO/X86/Inputs/struct-mapping-foo.ll
> test/ThinLTO/X86/struct-mapping.ll
>
> Index: test/ThinLTO/X86/struct-mapping.ll
> ===================================================================
> --- test/ThinLTO/X86/struct-mapping.ll
> +++ test/ThinLTO/X86/struct-mapping.ll
> @@ -0,0 +1,46 @@
> +; RUN: opt -module-summary %s -o %t1.bc
> +
> +; The output file name does matter, because it affetcs the function
> +; import order. This is the reason struct-mapping-foo.ll becomes %t3
> +; RUN: opt -module-summary %p/Inputs/struct-mapping-foo.ll -o %t3.bc
> +; RUN: opt -module-summary %p/Inputs/struct-mapping-baz.ll -o %t2.bc
> +
> +; RUN: llvm-lto -thinlto-action=thinlink %t1.bc %t2.bc %t3.bc -o %t4.index.bc
> +; RUN: llvm-lto -thinlto-action=import %t1.bc %t2.bc %t3.bc -thinlto-index=%t4.index.bc
> +; RUN: llvm-dis %t1.bc.thinlto.imported.bc -o - | FileCheck --check-prefix=IMPORT %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.
> +; IMPORT: %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 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/ThinLTO/X86/Inputs/struct-mapping-foo.ll
> ===================================================================
> --- test/ThinLTO/X86/Inputs/struct-mapping-foo.ll
> +++ test/ThinLTO/X86/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/ThinLTO/X86/Inputs/struct-mapping-baz.ll
> ===================================================================
> --- test/ThinLTO/X86/Inputs/struct-mapping-baz.ll
> +++ test/ThinLTO/X86/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;
> }
> @@ -682,6 +682,14 @@
> return NewGV;
> }
>
> +static StringRef getTypeNamePrefix(StringRef Name) {
> + size_t DotPos = Name.rfind('.');
> + return (DotPos == 0 || DotPos == StringRef::npos || Name.back() == '.' ||
> + !isdigit(static_cast<unsigned char>(Name[DotPos + 1])))
> + ? Name
> + : Name.substr(0, DotPos);
> +}
> +
> /// Loop over all of the linked values to compute type mappings. For example,
> /// if we link "extern Foo *x" and "Foo *x = NULL", then we have two struct
> /// types 'Foo' but one got renamed when the module was loaded into the same
> @@ -729,14 +737,12 @@
> }
>
> // Check to see if there is a dot in the name followed by a digit.
> - size_t DotPos = ST->getName().rfind('.');
> - if (DotPos == 0 || DotPos == StringRef::npos ||
> - ST->getName().back() == '.' ||
> - !isdigit(static_cast<unsigned char>(ST->getName()[DotPos + 1])))
> + auto STTypePrefix = getTypeNamePrefix(ST->getName());
> + if (STTypePrefix.size()== ST->getName().size())
> continue;
>
> // Check to see if the destination module has a struct with the prefix name.
> - StructType *DST = DstM.getTypeByName(ST->getName().substr(0, DotPos));
> + StructType *DST = DstM.getTypeByName(STTypePrefix);
> if (!DST)
> continue;
>
> @@ -901,7 +907,6 @@
> Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
> bool ForAlias) {
> GlobalValue *DGV = getLinkedToGlobal(SGV);
> -
> bool ShouldLink = shouldLink(DGV, *SGV);
>
> // just missing from map
> @@ -1409,12 +1414,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);
> @@ -1427,10 +1434,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);
> };
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list