[PATCH] D43199: [IRMover] Implement name based structure type mapping
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 13 07:04:32 PST 2018
On Tue, Feb 13, 2018 at 7:00 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:
> 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.
>
Hi Rafael,
See his explanation in the reply on https://reviews.llvm.org/D43077 from
early yesterday. It is leaving bitcasts that are blocking optimization.
Teresa
> 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
>
--
Teresa Johnson | Software Engineer | tejohnson at google.com | 408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180213/6f755956/attachment.html>
More information about the llvm-commits
mailing list