Teach ValueMapper to use ODR uniqued types when available

[tejohnson]

Summary

This is exposed during ThinLTO compilation, when we import an alias by
creating a clone of the aliasee. Without this fix the debug type is
unnecessarily cloned and we get a duplicate, undoing the uniquing.

Fixes PR36089.

[tejohnson]

ping

[pcc]

Should we instead change the ValueMapper to call DICompositeType::buildODRType if ODR uniquing is enabled?

[tejohnson]

In D41669#982160, @pcc wrote:

Should we instead change the ValueMapper to call DICompositeType::buildODRType if ODR uniquing is enabled?

We could. Confirmed change works instead of the other approach here (same test case). Let me know if you prefer that and I will change this patch.

diff --git a/lib/Transforms/Utils/ValueMapper.cpp b/lib/Transforms/Utils/ValueMapper.cpp
index 8c9ecbc3503..fc36501b145 100644

• a/lib/Transforms/Utils/ValueMapper.cpp

+++ b/lib/Transforms/Utils/ValueMapper.cpp
@@ -25,6 +25,7 @@
#include "llvm/IR/CallSite.h"
#include "llvm/IR/Constant.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h"
@@ -536,13 +537,30 @@ Optional<Metadata *> MDNodeMapper::tryToMapOperand(const Metadata *Op) {

return None;

}

+static Metadata *cloneOrBuildODR(const MDNode &N) {
+ auto *CT = dyn_cast_or_null<DICompositeType>(&N);
+ if (CT && CT->getIdentifier() != "") {
+ auto *NewCT = DICompositeType::buildODRType(
+ CT->getContext(),
+ *getCanonicalMDString(CT->getContext(), CT->getIdentifier()),
+ CT->getTag(), CT->getRawName(), CT->getFile(), CT->getLine(),
+ CT->getScope(), CT->getBaseType(), CT->getSizeInBits(),
+ CT->getAlignInBits(), CT->getOffsetInBits(), CT->getFlags(),
+ CT->getRawElements(), CT->getRuntimeLang(), CT->getVTableHolder(),
+ CT->getRawTemplateParams());
+ if (NewCT)
+ return NewCT;
+ }
+ return MDNode::replaceWithDistinct(N.clone());
+};
+
MDNode *MDNodeMapper::mapDistinctNode(const MDNode &N) {

assert(N.isDistinct() && "Expected a distinct node");
assert(!M.getVM().getMappedMD(&N) && "Expected an unmapped node");

• DistinctWorklist.push_back(cast<MDNode>(
• (M.Flags & RF_MoveDistinctMDs)
• ? M.mapToSelf(&N)
• : M.mapToMetadata(&N, MDNode::replaceWithDistinct(N.clone()))));

+ DistinctWorklist.push_back(
+ cast<MDNode>((M.Flags & RF_MoveDistinctMDs)
+ ? M.mapToSelf(&N)
+ : M.mapToMetadata(&N, cloneOrBuildODR(N))));

return DistinctWorklist.back();

}

[tejohnson]

Ping. It turns out this fixes https://bugs.llvm.org/show_bug.cgi?id=36089 (I tested the new version that uses the ValueMapper). @pcc: let me know if you prefer the ValueMapper approach (see my previous comments), and I will update the patch officially.

[pcc]

Sorry the value mapper version mostly looks good. One question.

if (CT && CT->getIdentifier() != "") {
+ auto *NewCT = DICompositeType::buildODRType

Can this be if (enableodruniquing() && CT && ...) Return CT;?

[tejohnson]

In D41669#992020, @pcc wrote:

Sorry the value mapper version mostly looks good. One question.

if (CT && CT->getIdentifier() != "") {
+ auto *NewCT = DICompositeType::buildODRType

Can this be if (enableodruniquing() && CT && ...) Return CT;?

Not sure what you mean - we don't want to return the source's CT. (And the first thing buildODRType does is check if ODR uniquing is enabled, so no need to check that here too).

I'll go ahead and update the patch with the new version.

[pcc]

But doesn't ODR type uniquing mean that there is only one CT for the whole LLVMContext?

[tejohnson]

In D41669#992036, @pcc wrote:

But doesn't ODR type uniquing mean that there is only one CT for the whole LLVMContext?

That's true, with ODR type uniquing we would have invoked buildODRType during the bitcode reading process, so it should be safe to use CT. I tested that with the test case here and in PR36089 and it works. Updating the patch in a min.

[tejohnson]

Update per comments

[pcc]

LGTM with nits

lib/Transforms/Utils/ValueMapper.cpp

541 ↗

(On Diff #132004)

I think this can be dyn_cast given that it is a reference.

548 ↗

(On Diff #132004)

Remove semicolon

[tejohnson]

address comments

[dexonsmith]

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

[tejohnson]

In D41669#2561267, @dexonsmith wrote:

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

I need to go back and look at this some more, unfortunately it's not immediately obvious. It looks like I was making this change for unspecified reasons related to my change to import aliases as a copy of the aliasee, and then found it fixed PR36089. Need to go back and see if the test case in that bug works now or still fails without this change.

Regarding CloneModule - from my skim of the other bug you are working on it appears that this is causing an issue when invoked while cloning a module when we split ThinLTO bitcode files - is that correct? In my case I was fixing an issue during importing, not during this cloning.

[dexonsmith]

In D41669#2563798, @tejohnson wrote:

In D41669#2561267, @dexonsmith wrote:

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

I need to go back and look at this some more, unfortunately it's not immediately obvious. It looks like I was making this change for unspecified reasons related to my change to import aliases as a copy of the aliasee, and then found it fixed PR36089. Need to go back and see if the test case in that bug works now or still fails without this change.

I'd bet the testcase in that bug will still fail without this change.

Instead of changing ValueMapper, maybe the right fix is similar to the code in CloneFunctionInto (which I'm conditionalizing in https://reviews.llvm.org/D96531, expecting to commit soon, just missed the window last week where I could watch the bots): prime the ValueToValueMap with mappings-to-self for any entry points to the metadata graph that shouldn't be cloned / duplicated.

Regarding CloneModule - from my skim of the other bug you are working on it appears that this is causing an issue when invoked while cloning a module when we split ThinLTO bitcode files - is that correct? In my case I was fixing an issue during importing, not during this cloning.

Yes, that's correct. Thanks for clarifying this is during importing; it's reasonable at a high level to reuse the ODR logic; the problem is that the value mapper code has more general usecases. (Seems like metadata ownership (especially for distinct nodes) needs to be cleaned up in some way... there are a bunch of subtleties that make it hard to reason about.)

Is the source module being discarded? If so, the importer could safely use RF_ReuseAndMutateDistinctMDs (the new name for RF_MoveDistinctMDs).

[tejohnson]

In D41669#2563866, @dexonsmith wrote:

In D41669#2563798, @tejohnson wrote:

In D41669#2561267, @dexonsmith wrote:

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

I need to go back and look at this some more, unfortunately it's not immediately obvious. It looks like I was making this change for unspecified reasons related to my change to import aliases as a copy of the aliasee, and then found it fixed PR36089. Need to go back and see if the test case in that bug works now or still fails without this change.

I'd bet the testcase in that bug will still fail without this change.

Actually - I just tried it and it passes with this change reverted. I guess something else already fixed that issue in a slightly different probably more correct way?

Instead of changing ValueMapper, maybe the right fix is similar to the code in CloneFunctionInto (which I'm conditionalizing in https://reviews.llvm.org/D96531, expecting to commit soon, just missed the window last week where I could watch the bots): prime the ValueToValueMap with mappings-to-self for any entry points to the metadata graph that shouldn't be cloned / duplicated.

Regarding CloneModule - from my skim of the other bug you are working on it appears that this is causing an issue when invoked while cloning a module when we split ThinLTO bitcode files - is that correct? In my case I was fixing an issue during importing, not during this cloning.

Yes, that's correct. Thanks for clarifying this is during importing; it's reasonable at a high level to reuse the ODR logic; the problem is that the value mapper code has more general usecases. (Seems like metadata ownership (especially for distinct nodes) needs to be cleaned up in some way... there are a bunch of subtleties that make it hard to reason about.)

Is the source module being discarded? If so, the importer could safely use RF_ReuseAndMutateDistinctMDs (the new name for RF_MoveDistinctMDs).

Yes it's being discarded. And yes it is already using RF_ReuseAndMutateDistinctMDs (set by default on the IRLinker constructor initialization of the ValueMapper). Hmm, now I'm confused. The case I modified in this patch is where this flag is not set. Aha - going through the debugger for the test case included with this patch shows that while the importer itself uses a ValueMapper with RF_ReuseAndMutateDistinctMDs enabled, the functionality that imports an alias as a copy of the aliasee (replaceAliasWithAliaseee) in fact uses CloneFunction/CloneFunctionInto, so a ValueMapper without that flag. So perhaps your fix in D96531 is sufficient and this change can be reverted?

[dexonsmith]

In D41669#2564112, @tejohnson wrote:

In D41669#2563866, @dexonsmith wrote:

In D41669#2563798, @tejohnson wrote:

In D41669#2561267, @dexonsmith wrote:

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

I need to go back and look at this some more, unfortunately it's not immediately obvious. It looks like I was making this change for unspecified reasons related to my change to import aliases as a copy of the aliasee, and then found it fixed PR36089. Need to go back and see if the test case in that bug works now or still fails without this change.

I'd bet the testcase in that bug will still fail without this change.

Actually - I just tried it and it passes with this change reverted. I guess something else already fixed that issue in a slightly different probably more correct way?

I'm seeing ThinLTO/X86/dicompositetype-unique-alias.ll fail locally. Maybe you didn't have 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a when you tried? (I probably hadn't pushed yet...)

Instead of changing ValueMapper, maybe the right fix is similar to the code in CloneFunctionInto (which I'm conditionalizing in https://reviews.llvm.org/D96531, expecting to commit soon, just missed the window last week where I could watch the bots): prime the ValueToValueMap with mappings-to-self for any entry points to the metadata graph that shouldn't be cloned / duplicated.

Regarding CloneModule - from my skim of the other bug you are working on it appears that this is causing an issue when invoked while cloning a module when we split ThinLTO bitcode files - is that correct? In my case I was fixing an issue during importing, not during this cloning.

Yes, that's correct. Thanks for clarifying this is during importing; it's reasonable at a high level to reuse the ODR logic; the problem is that the value mapper code has more general usecases. (Seems like metadata ownership (especially for distinct nodes) needs to be cleaned up in some way... there are a bunch of subtleties that make it hard to reason about.)

Is the source module being discarded? If so, the importer could safely use RF_ReuseAndMutateDistinctMDs (the new name for RF_MoveDistinctMDs).

Yes it's being discarded. And yes it is already using RF_ReuseAndMutateDistinctMDs (set by default on the IRLinker constructor initialization of the ValueMapper). Hmm, now I'm confused. The case I modified in this patch is where this flag is not set. Aha - going through the debugger for the test case included with this patch shows that while the importer itself uses a ValueMapper with RF_ReuseAndMutateDistinctMDs enabled, the functionality that imports an alias as a copy of the aliasee (replaceAliasWithAliaseee) in fact uses CloneFunction/CloneFunctionInto, so a ValueMapper without that flag. So perhaps your fix in D96531 is sufficient and this change can be reverted?

Your logic SGTM -- replaceAliasWithAliasee DOES use CloneFunction -- but as mentioned above the test is failing for me. I'm confused... maybe my tree is out-of-date, or maybe there's something subtle going on... I'll reply again if I figure it out.

[tejohnson]

In D41669#2564233, @dexonsmith wrote:

In D41669#2564112, @tejohnson wrote:

In D41669#2563866, @dexonsmith wrote:

In D41669#2563798, @tejohnson wrote:

In D41669#2561267, @dexonsmith wrote:

This is unsound, as it breaks CloneModule: it causes the original/cloned module to reference GlobalValues from the new module (composite types transitively reference global values when a type has "interesting" template parameters). See https://bugs.llvm.org/show_bug.cgi?id=48841 (especially https://bugs.llvm.org/show_bug.cgi?id=48841#c22, where I have a long explanation).

@tejohnson, can you share more context about why this was necessary? How is ThinLTO using the value mapper that this happens?

There should be another way to get this optimization that's sound, but I think it'll involve priming the ValueToValueMap that gets sent into MapValue / MapMetadata.

I need to go back and look at this some more, unfortunately it's not immediately obvious. It looks like I was making this change for unspecified reasons related to my change to import aliases as a copy of the aliasee, and then found it fixed PR36089. Need to go back and see if the test case in that bug works now or still fails without this change.

I'd bet the testcase in that bug will still fail without this change.

Actually - I just tried it and it passes with this change reverted. I guess something else already fixed that issue in a slightly different probably more correct way?

I'm seeing ThinLTO/X86/dicompositetype-unique-alias.ll fail locally. Maybe you didn't have 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a when you tried? (I probably hadn't pushed yet...)

Sorry to be unclear. That test case, added with this patch, does still fail for me with this change reverted. It will fail if there is more than one DICompositeType - please take a look to see if this looks problematic. I don't recall exactly why I was concerned about that originally. What doesn't fail for me anymore with this change reverted is the test case in the associated PR, which was an actual crash.

Note all of this was before you pushed 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a so didn't have that.

Instead of changing ValueMapper, maybe the right fix is similar to the code in CloneFunctionInto (which I'm conditionalizing in https://reviews.llvm.org/D96531, expecting to commit soon, just missed the window last week where I could watch the bots): prime the ValueToValueMap with mappings-to-self for any entry points to the metadata graph that shouldn't be cloned / duplicated.

Regarding CloneModule - from my skim of the other bug you are working on it appears that this is causing an issue when invoked while cloning a module when we split ThinLTO bitcode files - is that correct? In my case I was fixing an issue during importing, not during this cloning.

Yes, that's correct. Thanks for clarifying this is during importing; it's reasonable at a high level to reuse the ODR logic; the problem is that the value mapper code has more general usecases. (Seems like metadata ownership (especially for distinct nodes) needs to be cleaned up in some way... there are a bunch of subtleties that make it hard to reason about.)

Is the source module being discarded? If so, the importer could safely use RF_ReuseAndMutateDistinctMDs (the new name for RF_MoveDistinctMDs).

Yes it's being discarded. And yes it is already using RF_ReuseAndMutateDistinctMDs (set by default on the IRLinker constructor initialization of the ValueMapper). Hmm, now I'm confused. The case I modified in this patch is where this flag is not set. Aha - going through the debugger for the test case included with this patch shows that while the importer itself uses a ValueMapper with RF_ReuseAndMutateDistinctMDs enabled, the functionality that imports an alias as a copy of the aliasee (replaceAliasWithAliaseee) in fact uses CloneFunction/CloneFunctionInto, so a ValueMapper without that flag. So perhaps your fix in D96531 is sufficient and this change can be reverted?

Your logic SGTM -- replaceAliasWithAliasee DOES use CloneFunction -- but as mentioned above the test is failing for me. I'm confused... maybe my tree is out-of-date, or maybe there's something subtle going on... I'll reply again if I figure it out.

[dexonsmith]

In D41669#2564233, @dexonsmith wrote:

In D41669#2564112, @tejohnson wrote:

Yes it's being discarded. And yes it is already using RF_ReuseAndMutateDistinctMDs (set by default on the IRLinker constructor initialization of the ValueMapper). Hmm, now I'm confused. The case I modified in this patch is where this flag is not set. Aha - going through the debugger for the test case included with this patch shows that while the importer itself uses a ValueMapper with RF_ReuseAndMutateDistinctMDs enabled, the functionality that imports an alias as a copy of the aliasee (replaceAliasWithAliaseee) in fact uses CloneFunction/CloneFunctionInto, so a ValueMapper without that flag. So perhaps your fix in D96531 is sufficient and this change can be reverted?

Your logic SGTM -- replaceAliasWithAliasee DOES use CloneFunction -- but as mentioned above the test is failing for me. I'm confused... maybe my tree is out-of-date, or maybe there's something subtle going on... I'll reply again if I figure it out.

Looking at it in a debugger, I think I just need to update CloneFunctionInto to catch this case as well.

[dexonsmith]

In D41669#2564247, @tejohnson wrote:

Sorry to be unclear. That test case, added with this patch, does still fail for me with this change reverted. It will fail if there is more than one DICompositeType - please take a look to see if this looks problematic. I don't recall exactly why I was concerned about that originally. What doesn't fail for me anymore with this change reverted is the test case in the associated PR, which was an actual crash.

Note all of this was before you pushed 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a so didn't have that.

Aha, I get it; I was reading too fast before.

I think the test you added is a good one; better not to clone the composite type unnecessarily. I posted the "revert" at https://reviews.llvm.org/D96734, and it updates CloneFunctionInto to keep the testcase passing.

[tejohnson]

In D41669#2564314, @dexonsmith wrote:

In D41669#2564247, @tejohnson wrote:

Sorry to be unclear. That test case, added with this patch, does still fail for me with this change reverted. It will fail if there is more than one DICompositeType - please take a look to see if this looks problematic. I don't recall exactly why I was concerned about that originally. What doesn't fail for me anymore with this change reverted is the test case in the associated PR, which was an actual crash.

Note all of this was before you pushed 22a52dfddcefad4f275eb8ad1cc0e200074c2d8a so didn't have that.

Aha, I get it; I was reading too fast before.

I think the test you added is a good one; better not to clone the composite type unnecessarily. I posted the "revert" at https://reviews.llvm.org/D96734, and it updates CloneFunctionInto to keep the testcase passing.

Great, thanks for fixing this!

llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp

 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constant.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/GlobalObject.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
#endif
   }
 
   const MDNode &N = *cast<MDNode>(Op);
   if (N.isDistinct())
     return mapDistinctNode(N);
   return None;
 }
 
+static Metadata *cloneOrBuildODR(const MDNode &N) {
+  auto *CT = dyn_cast<DICompositeType>(&N);
+  // If ODR type uniquing is enabled, we would have uniqued composite types
+  // with identifiers during bitcode reading, so we can just use CT.
+  if (CT && CT->getContext().isODRUniquingDebugTypes() &&
+      CT->getIdentifier() != "")
+    return const_cast<DICompositeType *>(CT);
+  return MDNode::replaceWithDistinct(N.clone());
+}
+
 MDNode *MDNodeMapper::mapDistinctNode(const MDNode &N) {
   assert(N.isDistinct() && "Expected a distinct node");
   assert(!M.getVM().getMappedMD(&N) && "Expected an unmapped node");
-  DistinctWorklist.push_back(cast<MDNode>(
-      (M.Flags & RF_MoveDistinctMDs)
+  DistinctWorklist.push_back(
+      cast<MDNode>((M.Flags & RF_MoveDistinctMDs)
                        ? M.mapToSelf(&N)
-          : M.mapToMetadata(&N, MDNode::replaceWithDistinct(N.clone()))));
+                       : M.mapToMetadata(&N, cloneOrBuildODR(N))));
   return DistinctWorklist.back();
 }
 
 static ConstantAsMetadata *wrapConstantAsMetadata(const ConstantAsMetadata &CMD,
                                                   Value *MappedV) {
   if (CMD.getValue() == MappedV)
     return const_cast<ConstantAsMetadata *>(&CMD);
   return MappedV ? ConstantAsMetadata::getConstant(MappedV) : nullptr;

llvm/trunk/test/ThinLTO/X86/Inputs/dicompositetype-unique-alias.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+%struct.CFVS = type { %struct.Vec }
+%struct.Vec = type { i8 }
+%struct.S = type { i8 }
+
+@_ZN4CFVSD1Ev = alias void (%struct.CFVS*), void (%struct.CFVS*)* @_ZN4CFVSD2Ev
+
+define void @_ZN4CFVSD2Ev(%struct.CFVS* %this) unnamed_addr align 2 !dbg !8 {
+entry:
+  %this.addr = alloca %struct.CFVS*, align 8
+  store %struct.CFVS* %this, %struct.CFVS** %this.addr, align 8
+  %this1 = load %struct.CFVS*, %struct.CFVS** %this.addr, align 8
+  %m_val = getelementptr inbounds %struct.CFVS, %struct.CFVS* %this1, i32 0, i32 0
+  ret void
+}
+
+declare dereferenceable(1) %struct.S* @_Z3Getv()
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 6.0.0 (trunk 321360) (llvm/trunk 321359)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "bz188598-b.cpp", directory: "")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!8 = distinct !DISubprogram(name: "~CFVS", linkageName: "_ZN4CFVSD2Ev", scope: !9, file: !1, line: 2, type: !28, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: false, unit: !0, declaration: !27, variables: !2)
+!9 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "CFVS", file: !10, line: 7, size: 8, elements: !11, identifier: "_ZTS4CFVS")
+!10 = !DIFile(filename: "./bz188598.h", directory: "")
+!11 = !{!35}
+!27 = !DISubprogram(name: "~CFVS", scope: !9, file: !10, line: 8, type: !28, isLocal: false, isDefinition: false, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false)
+!28 = !DISubroutineType(types: !29)
+!29 = !{null, !30}
+!30 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!35 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)

llvm/trunk/test/ThinLTO/X86/dicompositetype-unique-alias.ll

+; RUN: opt -module-summary -o %t1.bc %s
+; RUN: opt -module-summary -o %t2.bc %S/Inputs/dicompositetype-unique-alias.ll
+; RUN: llvm-lto --thinlto-action=run %t1.bc %t2.bc -thinlto-save-temps=%t3.
+; RUN: llvm-dis %t3.0.3.imported.bc -o - | FileCheck %s
+; RUN: llvm-lto2 run %t1.bc %t2.bc -o %t --save-temps \
+; RUN: -r %t1.bc,_ZN1CD2Ev,pl \
+; RUN: -r %t1.bc,_ZN4CFVSD1Ev,l \
+; RUN: -r %t1.bc,_ZN4CFVSD2Ev,l \
+; RUN: -r %t1.bc,_Z3Getv,l \
+; RUN: -r %t2.bc,_ZN4CFVSD1Ev,pl \
+; RUN: -r %t2.bc,_ZN4CFVSD2Ev,pl \
+; RUN: -r %t2.bc,_Z3Getv,l
+; RUN: llvm-dis %t.1.3.import.bc -o - | FileCheck %s
+
+; Only llvm-lto2 adds the dso_local keyword, hence the {{.*}}
+; CHECK: define available_externally{{.*}} void @_ZN4CFVSD1Ev
+
+; Confirm that we only have a single DICompositeType after importing
+; both an alias and its aliasee, since ODR Type Uniquing is enabled.
+; CHECK: DICompositeType
+; CHECK-NOT: DICompositeType
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-scei-ps4"
+
+%class.C = type <{ i32 (...)**, %class.A, %struct.CFVS, [6 x i8] }>
+%class.A = type { %struct.Vec }
+%struct.Vec = type { i8 }
+%struct.CFVS = type { %struct.Vec }
+%struct.S = type { i8 }
+
+define void @_ZN1CD2Ev(%class.C* %this) unnamed_addr align 2 {
+entry:
+  %this.addr = alloca %class.C*, align 8
+  %this1 = load %class.C*, %class.C** %this.addr, align 8
+  %m = getelementptr inbounds %class.C, %class.C* %this1, i32 0, i32 2
+  call void @_ZN4CFVSD1Ev(%struct.CFVS* %m), !dbg !50
+  call void @_ZN4CFVSD2Ev(%struct.CFVS* %m), !dbg !50
+  ret void
+}
+
+declare void @_ZN4CFVSD1Ev(%struct.CFVS*) unnamed_addr
+declare void @_ZN4CFVSD2Ev(%struct.CFVS*) unnamed_addr
+
+declare dereferenceable(1) %struct.S* @_Z3Getv()
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 6.0.0 (trunk 321360) (llvm/trunk 321359)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "bz188598-a.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 2}
+!6 = !{i32 7, !"PIC Level", i32 2}
+!8 = distinct !DISubprogram(name: "~C", linkageName: "_ZN1CD2Ev", scope: !1, file: !1, line: 9, type: !47, isLocal: false, isDefinition: true, scopeLine: 9, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
+!47 = !DISubroutineType(types: !48)
+!48 = !{!55}
+!50 = !DILocation(line: 9, scope: !51)
+!51 = distinct !DILexicalBlock(scope: !8, file: !1, line: 9)
+!55 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)