[llvm] r325686 - [IRMover] Implement name based structure type mapping
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 21 12:12:19 PST 2018
I never LGTMed, I just provided a set of initial comments.
Reverting for now as it is still not clear that we can solve this
without adding adding more name based logic.
Cheers,
Rafael
Eugene Leviant via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: evgeny777
> Date: Wed Feb 21 07:13:48 2018
> New Revision: 325686
>
> URL: http://llvm.org/viewvc/llvm-project?rev=325686&view=rev
> Log:
> [IRMover] Implement name based structure type mapping
>
> Differential revision: https://reviews.llvm.org/D43199
>
> Added:
> llvm/trunk/test/Linker/Inputs/struct-mapping.ll
> llvm/trunk/test/Linker/struct-mapping.ll
> Modified:
> llvm/trunk/include/llvm/Linker/IRMover.h
> llvm/trunk/lib/Linker/IRMover.cpp
> llvm/trunk/tools/llvm-link/llvm-link.cpp
>
> Modified: llvm/trunk/include/llvm/Linker/IRMover.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Linker/IRMover.h?rev=325686&r1=325685&r2=325686&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Linker/IRMover.h (original)
> +++ llvm/trunk/include/llvm/Linker/IRMover.h Wed Feb 21 07:13:48 2018
> @@ -49,17 +49,23 @@ public:
> // The set of opaque types is the composite module.
> DenseSet<StructType *> OpaqueStructTypes;
>
> - // The set of identified but non opaque structures in the composite module.
> - DenseSet<StructType *, StructTypeKeyInfo> NonOpaqueStructTypes;
> -
> - public:
> - void addNonOpaque(StructType *Ty);
> - void switchToNonOpaque(StructType *Ty);
> - void addOpaque(StructType *Ty);
> - StructType *findNonOpaque(ArrayRef<Type *> ETypes, bool IsPacked);
> - bool hasType(StructType *Ty);
> - };
> -
> + // 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,
> + StringRef Name);
> + bool hasType(StructType *Ty);
> + };
> +
> IRMover(Module &M);
>
> typedef std::function<void(GlobalValue &)> ValueAdder;
>
> Modified: llvm/trunk/lib/Linker/IRMover.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=325686&r1=325685&r2=325686&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Linker/IRMover.cpp (original)
> +++ llvm/trunk/lib/Linker/IRMover.cpp Wed Feb 21 07:13:48 2018
> @@ -318,8 +318,8 @@ Type *TypeMapTy::get(Type *Ty, SmallPtrS
> return *Entry = Ty;
> }
>
> - if (StructType *OldT =
> - DstStructTypesSet.findNonOpaque(ElementTypes, IsPacked)) {
> + if (StructType *OldT = DstStructTypesSet.findNonOpaque(
> + ElementTypes, IsPacked, STy->getName())) {
> STy->setName("");
> return *Entry = OldT;
> }
> @@ -906,7 +906,6 @@ bool IRLinker::shouldLink(GlobalValue *D
> Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
> bool ForAlias) {
> GlobalValue *DGV = getLinkedToGlobal(SGV);
> -
> bool ShouldLink = shouldLink(DGV, *SGV);
>
> // just missing from map
> @@ -1410,12 +1409,14 @@ bool IRMover::StructTypeKeyInfo::isEqual
>
> 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);
> @@ -1428,10 +1429,16 @@ void IRMover::IdentifiedStructTypeSet::a
>
> 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) {
>
> Added: llvm/trunk/test/Linker/Inputs/struct-mapping.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/struct-mapping.ll?rev=325686&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/Inputs/struct-mapping.ll (added)
> +++ llvm/trunk/test/Linker/Inputs/struct-mapping.ll Wed Feb 21 07:13:48 2018
> @@ -0,0 +1,4 @@
> +%struct.Baz = type { i64, i64, %struct.Foo }
> +%struct.Foo = type { i64, i64 }
> +
> + at baz = global %struct.Baz zeroinitializer
>
> Added: llvm/trunk/test/Linker/struct-mapping.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/struct-mapping.ll?rev=325686&view=auto
> ==============================================================================
> --- llvm/trunk/test/Linker/struct-mapping.ll (added)
> +++ llvm/trunk/test/Linker/struct-mapping.ll Wed Feb 21 07:13:48 2018
> @@ -0,0 +1,12 @@
> +; RUN: llvm-link --initial-module=%s %p/Inputs/struct-mapping.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 }
> +
> +%struct.Bar = type { i64, i64 }
> +%struct.Foo = type { i64, i64 }
> +
> + at bar = global %struct.Bar zeroinitializer
> + at foo = global %struct.Foo zeroinitializer
>
> Modified: llvm/trunk/tools/llvm-link/llvm-link.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=325686&r1=325685&r2=325686&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-link/llvm-link.cpp (original)
> +++ llvm/trunk/tools/llvm-link/llvm-link.cpp Wed Feb 21 07:13:48 2018
> @@ -70,6 +70,11 @@ static cl::opt<std::string>
> 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 @@ int main(int argc, char **argv) {
> 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;
>
>
> _______________________________________________
> 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