Support intrinsic overloading on unnamed types

[jeroen.dobbelaere]

Summary

This patch adds support for intrinsic overloading on unnamed types.

This fixes PR38117 and PR48340 and will also be needed for the Full Restrict Patches (D68484).

The main problem is that the intrinsic overloading name mangling is using 's_s' for unnamed types.
This can result in identical intrinsic mangled names for different function prototypes.

This patch changes this by adding a '.XXXXX' to the intrinsic mangled name when at least one of the types is based on an unnamed type, ensuring that we get a unique name.

Implementation details:

• The mapping is created on demand and kept in Module.
• It also checks for existing clashes and recycles potentially existing prototypes and declarations.
• Because of extra data in Module, Intrinsic::getName needs an extra Module* argument and, for speed, an optional FunctionType* argument.
• I still kept the original two-argument 'Intrinsic::getName' around which keeps the original behavior (providing the base name).
  • Main reason is that I did not want to change the LLVMIntrinsicGetName version, as I don't know how acceptable such a change is
  • The current situation already has a limitation. So that should not get worse with this patch.
• Intrinsic::getDeclaration and the verifier are now using the new version.

Other notes:

• As far as I see, this should not suffer from stability issues. The count is only added for prototypes depending on at least one anonymous struct
• The initial count starts from 0 for each intrinsic mangled name.
• In case of name clashes, existing prototypes are remembered and reused when that makes sense.

[fhahn]

Is this the same idea as in D48541 (linked from PR38117), where we use LLVMContext to keep track of IDs for anonymous types? If that's the case, the same objections as for D48541 probably apply here as well. I think one of the problems is that the names we assign depend on the order different modules are loaded.

[jeroen.dobbelaere]

In D91250#2388377, @fhahn wrote:

Is this the same idea as in D48541 (linked from PR38117), where we use LLVMContext to keep track of IDs for anonymous types? If that's the case, the same objections as for D48541 probably apply here as well. I think one of the problems is that the names we assign depend on the order different modules are loaded.

Somehow I missed that one before.. But yes, the current implementation will suffer the same problem. Moving the DenseMap from LLVMContext to Module should resolve that problem ?

[jeroen.dobbelaere]

This new version uses a different approach: we now work by 'uniquifying' the initial BaseName of an intrinsic mangled named when it is based on an anonymous struct.

As an example, the testcase from D68484#2372356 now results in:

%0 = type { i32 }
%1 = type { i32 }

define void @bar01(%0* %ptr0, %1* %ptr1) {
  %1 = call i8* @llvm.noalias.decl.p0i8.p0p0s_s.i64.0(%0** null, i64 0, metadata !0)
  %2 = call %0* @llvm.noalias.p0s_s.p0i8.p0p0s_s.i64.0(%0* %ptr0, i8* %1, %0** null, i64 0, metadata !0)
  store %0 zeroinitializer, %0* %2, align 4, !noalias !0
  %3 = call i8* @llvm.noalias.decl.p0i8.p0p0s_s.i64.1(%1** null, i64 0, metadata !3)
  %4 = call %1* @llvm.noalias.p0s_s.p0i8.p0p0s_s.i64.1(%1* %ptr1, i8* %3, %1** null, i64 0, metadata !3)
  store %1 zeroinitializer, %1* %4, align 4, !noalias !3
  ret void
}

; Function Attrs: argmemonly nounwind
declare i8* @llvm.noalias.decl.p0i8.p0p0s_s.i64.0(%0**, i64, metadata) #0

; Function Attrs: argmemonly nounwind speculatable
declare %0* @llvm.noalias.p0s_s.p0i8.p0p0s_s.i64.0(%0*, i8*, %0**, i64, metadata) #1

; Function Attrs: argmemonly nounwind
declare i8* @llvm.noalias.decl.p0i8.p0p0s_s.i64.1(%1**, i64, metadata) #0

; Function Attrs: argmemonly nounwind speculatable
declare %1* @llvm.noalias.p0s_s.p0i8.p0p0s_s.i64.1(%1*, i8*, %1**, i64, metadata) #1

[jeroen.dobbelaere]

I would like to propose this version for review. As far as I am aware, this version should not suffer from the objections made in D48541. Also a testcase has been included.

[nikic]

Can't really comment on whether this makes sense approach wise. For testing, I'd suggest to test that this is correctly handled in bitcode serialization/loading as well, and to drop the current hack in PredicateInfo and see if it works fine for that use case.

[jeroen.dobbelaere]

In D91250#2400619, @nikic wrote:

... For testing, I'd suggest to test that this is correctly handled in bitcode serialization/loading as well...

The test should be doing that already by assembling and passing the bitcode to llvm-dis ?

[fhahn]

I think we would also need tests for combining multiple modules with intrinsics with unnamed type.

If the module owns the numbering, can encounter name/type clashes, e.g. if 2 modules have unnamed types that are different. i.e. both have declare %0* @llvm.ssa.copy.p0s_s.0(%0* returned) ,but %0 is defined as different types in both modules ?

[jeroen.dobbelaere]

In D91250#2402087, @fhahn wrote:

I think we would also need tests for combining multiple modules with intrinsics with unnamed type.

If the module owns the numbering, can encounter name/type clashes, e.g. if 2 modules have unnamed types that are different. i.e. both have declare %0* @llvm.ssa.copy.p0s_s.0(%0* returned) ,but %0 is defined as different types in both modules ?

This is indeed a good suggestion. I am adding a link test (and it does bring up a case that I did not foresee, so I am also fixing it).

[jeroen.dobbelaere]

I improved the patch to actually support linking.

[jeroen.dobbelaere]

Moved the call to 'forceRenaming' closer to the original location.

[jeroen.dobbelaere]

ping

[efriedma]

The approach looks reasonable. Please ensure this this documented in LangRef.

[jeroen.dobbelaere]

Updated langref. Rebased.

[jeroen.dobbelaere]

ping

[fhahn]

llvm/docs/LangRef.rst

20074

submit the llvm.ssa.copy fixes separately?

llvm/include/llvm/IR/Module.h

202

nit: period at end.

204

Where those the name come in here? It maps intrinsic ids to other ids?

llvm/lib/IR/Function.cpp

809

can we assert that there are no unnamed types involved here?

[jeroen.dobbelaere]

I am doing a testrun with the assert. Based on that outcome I'll update the patches.

llvm/docs/LangRef.rst

20074

ok. I can split it out.

llvm/include/llvm/IR/Module.h

202

nit: period at end.

What is the preference ? the comments before are also mixed wrt to the period.

204

Where those the name come in here? It maps intrinsic ids to other ids?

This map, based on the intrinsic ID and the FunctionType, allow us to remember what extension number has been associated to that combination. The uniqued name is based on the intrinsic ID, the FunctionType and that extension number.

llvm/lib/IR/Function.cpp

809

Should be possible. I would add the assert at line 796-797.

[jeroen.dobbelaere]

Taking comments into account:

• doc update
• added assertion

[jeroen.dobbelaere]

Hi @fhahn, @efriedma is there anything else that should be modified or is this ok to go ?

[jeroen.dobbelaere]

Ping

[nikic]

This patch also fixes https://bugs.llvm.org/show_bug.cgi?id=48340, you might want to add a test case for that.

[jeroen.dobbelaere]

• added reference to and testcase for PR48340
• rebased

[fhahn]

Thanks for the updates. I think this basically looks good now, just another round of smallish comments.

llvm/docs/LangRef.rst

11402

nit: Perhaps break up the sentence and mention that it allows differentiating intrinsics that have different unnamed types as arguments?

llvm/include/llvm/IR/Intrinsics.h

66

nit: Perhaps it's simpler to just say something like a function type \p FT can be provided to avoid computing it..

69

nit: llvm:: should not be needed.

llvm/lib/IR/Function.cpp

722

nit: I think it would be more explicit to return a pair with a bool, rather than having this out parameter. But. I guess the implementation would get a bit messier in the function.

799

nit: checking explicitly for == nullptr is very uncommon in LLVM, can you just check for FT/!FT?

llvm/lib/IR/Module.cpp

478

nit: no need to capture everything, it should be enough to capture BaseName?

Also, please fix the tidy warning.

502

nit: uncommon in LLVM to check == nullptr.

llvm/test/Transforms/LoopVectorize/pr48340.ll

3 ↗

(On Diff #329310)

not needed.

4 ↗

(On Diff #329310)

not needed.

6 ↗

(On Diff #329310)

Is this required to the test? If so, please move in the X86 subdirectory.

13 ↗

(On Diff #329310)

You could also check for the vector.body: block.

20 ↗

(On Diff #329310)

is it possible to get rid for the undefs in the tests? (and also have named values for the basic blocks at least?

53 ↗

(On Diff #329310)

is this needed?

55 ↗

(On Diff #329310)

is this needed?

[jeroen.dobbelaere]

Changed according to comments.

[jeroen.dobbelaere]

The pr48340.ll test was also moved to the X86 directory.

[fhahn]

LGTM, thanks! This patch should address the main issue earlier patches had, which were not handling name clashes on combining modules properly. Please wait a couple of days with committing, in cases there are any other comments.

llvm/docs/LangRef.rst

11405

nit: in the module?

11407

nit: changed how? new numbers will get assigned, right?

llvm/include/llvm/IR/Module.h

202

My preference is . at the end, but I am not sure if there's any rule.

204

Sorry, I meant the Name part in the name of the variable UniquedIntrinsicNames. The variable does not contain the full names, just the ids for intrinsic::id, type combinations. Not sure if there's a better name.

llvm/lib/IR/Function.cpp

809

Sounds good.

llvm/test/Transforms/LoopVectorize/X86/pr48340.ll

1 ↗

(On Diff #329756)

can you add -force-vector-width=4 -force-vector-interleave=1 or some similar settings? That way the test should be more robust with respect to cost-model changes.

55 ↗

(On Diff #329756)

nit: stray newline.

[jeroen.dobbelaere]

Thank you for all the help with the review !

[jeroen.dobbelaere]

Rebased. Modified langref and test according to comments.

[fhahn]

It looks like this causes assertion failures when dis-assembling some bitcode https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32288

[jeroen.dobbelaere]

In D91250#2639731, @fhahn wrote:

It looks like this causes assertion failures when dis-assembling some bitcode https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32288

Do you have a access to a full backtrace ? I am not able to access the 'detailed report' and it will probably be hard for me to find out on how the
set up a docker image on the corporate system.'

One possible problem could be when LLVMIntrinsicCopyOverloadedName is used with an unnamed type. Not sure what guarantees we can give there.

[fhahn]

In D91250#2639837, @jeroen.dobbelaere wrote:

In D91250#2639731, @fhahn wrote:

It looks like this causes assertion failures when dis-assembling some bitcode https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32288

Do you have a access to a full backtrace ? I am not able to access the 'detailed report' and it will probably be hard for me to find out on how the
set up a docker image on the corporate system.'

One possible problem could be when LLVMIntrinsicCopyOverloadedName is used with an unnamed type. Not sure what guarantees we can give there.

You should be able to download the reproducer. Running LLVM-dis should crash. https://oss-fuzz.com/download?testcase_id=5766959267446784

[jeroen.dobbelaere]

In D91250#2639839, @fhahn wrote:

You should be able to download the reproducer. Running LLVM-dis should crash. https://oss-fuzz.com/download?testcase_id=5766959267446784

Ok. I am able to reproduce it now. It seems llvm.lifetime.end can also depend on unnamed types. I'll go over the various cases in AutoUpgrade.cpp and try to come up with a fix tomorrow.

Thanks for the notification !

llvm/docs/LangRef.rst

 
 .. _t_opaque:
 
 Opaque Structure Types
 """"""""""""""""""""""
 
 :Overview:
 
-Opaque structure types are used to represent named structure types that
+Opaque structure types are used to represent structure types that
 do not have a body specified. This corresponds (for example) to the C
-notion of a forward declared structure.
+notion of a forward declared structure. They can be named (``%X``) or
+unnamed (``%52``).
 
 :Syntax:
 
 ::
 
       %X = type opaque
       %52 = type opaque
 

 ``llvm.ctpop`` function can take an integer of any width and returns an
 integer of exactly the same integer width. This leads to a family of
 functions such as ``i8 @llvm.ctpop.i8(i8 %val)`` and
 ``i29 @llvm.ctpop.i29(i29 %val)``. Only one type, the return type, is
 overloaded, and only one type suffix is required. Because the argument's
 type is matched against the return type, it does not require its own
 name suffix.
 
+:ref:`Unnamed types <t_opaque>` are encoded as ``s_s``. Overloaded intrinsics
+that depend on an unnamed type in one of its overloaded argument types get an
+additional ``.<number>`` suffix, that allows to differentiate such intrinsics.
+(For example: ``llvm.ssa.copy.p0s_s.2(%42*)``) The number is tracked in the
+LLVM module and it ensures unique names in a module. While linking together two
+modules, it is still possible to get a name clash. In that case one of the
+names will be changed.
+
 For target developers who are defining intrinsics for back-end code
 generation, any intrinsic overloads based solely the distinction between
 integer or floating point types should not be relied upon for correct
 code generation. In such cases, the recommended approach for target
 maintainers when defining intrinsics is to create separate integer and
 FP intrinsics rather than rely on overloading. For example, if different
 codegen is required for ``llvm.target.foo(<4 x i32>)`` and
 ``llvm.target.foo(<4 x float>)`` then these should be split into

 if the extra information provided by the ``llvm.assume`` intrinsic does not cause
 sufficient overall improvement in code quality. For this reason,
 ``llvm.assume`` should not be used to document basic mathematical invariants
 that the optimizer can otherwise deduce or facts that are of little use to the
 optimizer.
 
 .. _int_ssa_copy:
 
-'``llvm.ssa_copy``' Intrinsic
+'``llvm.ssa.copy``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
 
 ::
 
-      declare type @llvm.ssa_copy(type %operand) returned(1) readnone
+      declare type @llvm.ssa.copy(type %operand) returned(1) readnone
 
 Arguments:
 """"""""""
 
 The first argument is an operand which is used as the returned value.
 
 Overview:
 """"""""""
 
-The ``llvm.ssa_copy`` intrinsic can be used to attach information to
+The ``llvm.ssa.copy`` intrinsic can be used to attach information to
 operations by copying them and giving them new names.  For example,
 the PredicateInfo utility uses it to build Extended SSA form, and
 attach various forms of information to operands that dominate specific
 uses.  It is not meant for general use, only for building temporary
 renaming forms that require value splits at certain points.
 
 .. _type.test:
 

llvm/include/llvm/IR/Intrinsics.h

#undef GET_INTRINSIC_ENUM_VALUES
   };
 
   /// Return the LLVM name for an intrinsic, such as "llvm.ppc.altivec.lvx".
   /// Note, this version is for intrinsics with no overloads.  Use the other
   /// version of getName if overloads are required.
   StringRef getName(ID id);
 
   /// Return the LLVM name for an intrinsic, such as "llvm.ppc.altivec.lvx".
-  /// Note, this version of getName supports overloads, but is less efficient
-  /// than the StringRef version of this function.  If no overloads are
-  /// requried, it is safe to use this version, but better to use the StringRef
-  /// version.
-  std::string getName(ID id, ArrayRef<Type*> Tys);
+  /// Note, this version of getName supports overloads, but not unnamed types.
+  /// It is less efficient than the StringRef version of this function. If no
+  /// overloads are required, it is safe to use this version, but better to use
+  /// the StringRef version.
+  std::string getName(ID id, ArrayRef<Type *> Tys);
 
+  /// Return the LLVM name for an intrinsic, such as "llvm.ssa.copy.p0s_s.1".
+  /// Note, this version of getName supports overloads and unnamed types, but is
+  /// less efficient than the StringRef version of this function.  If no
+  /// overloads are required, it is safe to use this version, but better to use
+  /// the StringRef version. The function type can be provided as a speedup.
+  /// It is used (or computed) if one of the types is based on an unnamed type.
+  std::string getName(ID id, ArrayRef<Type *> Tys, Module *M,
+                      llvm::FunctionType *FT);
+
   /// Return the function type for an intrinsic.
   FunctionType *getType(LLVMContext &Context, ID id,
                         ArrayRef<Type*> Tys = None);
 
   /// Returns true if the intrinsic can be overloaded.
   bool isOverloaded(ID id);
 
   /// Returns true if the intrinsic is a leaf, i.e. it does not make any calls

llvm/include/llvm/IR/Module.h

private:
   Materializer;                   ///< Used to materialize GlobalValues
   std::string ModuleID;           ///< Human readable identifier for the module
   std::string SourceFileName;     ///< Original source file name for module,
                                   ///< recorded in bitcode.
   std::string TargetTriple;       ///< Platform target triple Module compiled on
                                   ///< Format: (arch)(sub)-(vendor)-(sys0-(abi)
   NamedMDSymTabType NamedMDSymTab;  ///< NamedMDNode names.
   DataLayout DL;                  ///< DataLayout associated with the module
+  StringMap<unsigned>
+      CurrentIntrinsicIds; ///< Keep track of the current unique id count for
+                           ///< the specified intrinsic basename
+  DenseMap<std::pair<Intrinsic::ID, const FunctionType *>, unsigned>
+      UniquedIntrinsicNames; ///< Keep track of uniqued names of intrinsics
+                             ///< based on unnamed types
 
   friend class Constant;
 
 /// @}
 /// @name Constructors
 /// @{
 public:
   /// The Module constructor. Note that there is no default constructor. You
/// @{
 
   /// Populate client supplied SmallVector with the bundle tags registered in
   /// this LLVMContext.  The bundle tags are ordered by increasing bundle IDs.
   /// \see LLVMContext::getOperandBundleTagID
   void getOperandBundleTags(SmallVectorImpl<StringRef> &Result) const;
 
   std::vector<StructType *> getIdentifiedStructTypes() const;
 
+  /// Return a unique name for an intrinsic whose mangling is based on an
+  /// unnamed type. The Proto represents the function prototype.
+  std::string getUniqueIntrinsicName(StringRef BaseName, Intrinsic::ID id,
+                                     const FunctionType *Proto);
+
 /// @}
 /// @name Function Accessors
 /// @{
 
   /// Look up the specified function in the module symbol table. Four
   /// possibilities:
   ///   1. If it does not exist, add a prototype for the function and return it.
   ///   2. Otherwise, if the existing function has the correct prototype, return

llvm/lib/IR/Function.cpp

 /// of named types is simply their name.  Manglings for unnamed types consist
 /// of a prefix ('p' for pointers, 'a' for arrays, 'f_' for functions)
 /// combined with the mangling of their component types.  A vararg function
 /// type will have a suffix of 'vararg'.  Since function types can contain
 /// other function types, we close a function type mangling with suffix 'f'
 /// which can't be confused with it's prefix.  This ensures we don't have
 /// collisions between two unrelated function types. Otherwise, you might
 /// parse ffXX as f(fXX) or f(fX)X.  (X is a placeholder for any other type.)
-///
-static std::string getMangledTypeStr(Type* Ty) {
+/// The HasUnnamedType boolean is set if an unnamed type was encountered,
+/// indicating that extra care must be taken to ensure a unique name.
+static std::string getMangledTypeStr(Type *Ty, bool &HasUnnamedType) {
   std::string Result;
   if (PointerType* PTyp = dyn_cast<PointerType>(Ty)) {
     Result += "p" + utostr(PTyp->getAddressSpace()) +
-      getMangledTypeStr(PTyp->getElementType());
+              getMangledTypeStr(PTyp->getElementType(), HasUnnamedType);
   } else if (ArrayType* ATyp = dyn_cast<ArrayType>(Ty)) {
     Result += "a" + utostr(ATyp->getNumElements()) +
-      getMangledTypeStr(ATyp->getElementType());
+              getMangledTypeStr(ATyp->getElementType(), HasUnnamedType);
   } else if (StructType *STyp = dyn_cast<StructType>(Ty)) {
     if (!STyp->isLiteral()) {
       Result += "s_";
+      if (STyp->hasName())
         Result += STyp->getName();
+      else
+        HasUnnamedType = true;
     } else {
       Result += "sl_";
       for (auto Elem : STyp->elements())
-        Result += getMangledTypeStr(Elem);
+        Result += getMangledTypeStr(Elem, HasUnnamedType);
     }
     // Ensure nested structs are distinguishable.
     Result += "s";
   } else if (FunctionType *FT = dyn_cast<FunctionType>(Ty)) {
-    Result += "f_" + getMangledTypeStr(FT->getReturnType());
+    Result += "f_" + getMangledTypeStr(FT->getReturnType(), HasUnnamedType);
     for (size_t i = 0; i < FT->getNumParams(); i++)
-      Result += getMangledTypeStr(FT->getParamType(i));
+      Result += getMangledTypeStr(FT->getParamType(i), HasUnnamedType);
     if (FT->isVarArg())
       Result += "vararg";
     // Ensure nested function types are distinguishable.
     Result += "f";
   } else if (VectorType* VTy = dyn_cast<VectorType>(Ty)) {
     ElementCount EC = VTy->getElementCount();
     if (EC.isScalable())
       Result += "nx";
     Result += "v" + utostr(EC.getKnownMinValue()) +
-              getMangledTypeStr(VTy->getElementType());
+              getMangledTypeStr(VTy->getElementType(), HasUnnamedType);
   } else if (Ty) {
     switch (Ty->getTypeID()) {
     default: llvm_unreachable("Unhandled type");
     case Type::VoidTyID:      Result += "isVoid";   break;
     case Type::MetadataTyID:  Result += "Metadata"; break;
     case Type::HalfTyID:      Result += "f16";      break;
     case Type::BFloatTyID:    Result += "bf16";     break;
     case Type::FloatTyID:     Result += "f32";      break;

 
 StringRef Intrinsic::getName(ID id) {
   assert(id < num_intrinsics && "Invalid intrinsic ID!");
   assert(!Intrinsic::isOverloaded(id) &&
          "This version of getName does not support overloading");
   return IntrinsicNameTable[id];
 }
 
-std::string Intrinsic::getName(ID id, ArrayRef<Type*> Tys) {
+std::string Intrinsic::getName(ID id, ArrayRef<Type *> Tys, Module *M,
+                               FunctionType *FT) {
   assert(id < num_intrinsics && "Invalid intrinsic ID!");
   assert((Tys.empty() || Intrinsic::isOverloaded(id)) &&
          "This version of getName is for overloaded intrinsics only");
+  bool HasUnnamedType = false;
   std::string Result(IntrinsicNameTable[id]);
   for (Type *Ty : Tys) {
-    Result += "." + getMangledTypeStr(Ty);
+    Result += "." + getMangledTypeStr(Ty, HasUnnamedType);
+  }
+  if (M && HasUnnamedType) {
+    if (FT == nullptr)
+      FT = getType(M->getContext(), id, Tys);
+    else
+      assert((FT == getType(M->getContext(), id, Tys)) &&
+             "Provided FunctionType must match arguments");
+    return M->getUniqueIntrinsicName(Result, id, FT);
   }
   return Result;
 }
 
+std::string Intrinsic::getName(ID id, ArrayRef<Type *> Tys) {
+  return getName(id, Tys, nullptr, nullptr);
+}
+
 /// IIT_Info - These are enumerators that describe the entries returned by the
 /// getIntrinsicInfoTableEntries function.
 ///
 /// NOTE: This must be kept in synch with the copy in TblGen/IntrinsicEmitter!
 enum IIT_Info {
   // Common values should be encoded with 0-15.
   IIT_Done = 0,
   IIT_I1   = 1,

 /// This defines the "Intrinsic::getAttributes(ID id)" method.
 #define GET_INTRINSIC_ATTRIBUTES
 #include "llvm/IR/IntrinsicImpl.inc"
 #undef GET_INTRINSIC_ATTRIBUTES
 
 Function *Intrinsic::getDeclaration(Module *M, ID id, ArrayRef<Type*> Tys) {
   // There can never be multiple globals with the same name of different types,
   // because intrinsics must be a specific type.
+  auto *FT = getType(M->getContext(), id, Tys);
   return cast<Function>(
-      M->getOrInsertFunction(Tys.empty() ? getName(id) : getName(id, Tys),
+      M->getOrInsertFunction(Tys.empty() ? getName(id)
+                                         : getName(id, Tys, M, FT),
                              getType(M->getContext(), id, Tys))
           .getCallee());
 }
 
 // This defines the "Intrinsic::getIntrinsicForGCCBuiltin()" method.
 #define GET_LLVM_INTRINSIC_FOR_GCC_BUILTIN
 #include "llvm/IR/IntrinsicImpl.inc"
 #undef GET_LLVM_INTRINSIC_FOR_GCC_BUILTIN

 
 Optional<Function *> Intrinsic::remangleIntrinsicFunction(Function *F) {
   SmallVector<Type *, 4> ArgTys;
   if (!getIntrinsicSignature(F, ArgTys))
     return None;
 
   Intrinsic::ID ID = F->getIntrinsicID();
   StringRef Name = F->getName();
-  if (Name == Intrinsic::getName(ID, ArgTys))
+  if (Name ==
+      Intrinsic::getName(ID, ArgTys, F->getParent(), F->getFunctionType()))
     return None;
 
   auto NewDecl = Intrinsic::getDeclaration(F->getParent(), ID, ArgTys);
   NewDecl->setCallingConv(F->getCallingConv());
   assert(NewDecl->getFunctionType() == F->getFunctionType() &&
          "Shouldn't change the signature");
   return NewDecl;
 }

llvm/lib/IR/Module.cpp

std::vector<StructType *> Module::getIdentifiedStructTypes() const {
 
   std::vector<StructType *> Ret;
   TypeFinder SrcStructTypes;
   SrcStructTypes.run(*this, true);
   Ret.assign(SrcStructTypes.begin(), SrcStructTypes.end());
   return Ret;
 }
 
+std::string Module::getUniqueIntrinsicName(StringRef BaseName, Intrinsic::ID id,
+                                           const FunctionType *Proto) {
+  auto encode = [&](unsigned Suffix) {
+    return (Twine(BaseName) + "." + Twine(Suffix)).str();
+  };
+
+  {
+    // fast path - the prototype is already known
+    auto UinItInserted = UniquedIntrinsicNames.insert({{id, Proto}, 0});
+    if (!UinItInserted.second)
+      return encode(UinItInserted.first->second);
+  }
+
+  // Not known yet. A new entry was created with index 0. Check if there already
+  // exists a matching declaration, or select a new entry.
+
+  // Start looking for names with the current known maximum count (or 0).
+  auto NiidItInserted = CurrentIntrinsicIds.insert({BaseName, 0});
+  unsigned Count = NiidItInserted.first->second;
+
+  // This might be slow if a whole population of intrinsics already existed, but
+  // we cache the values for later usage.
+  std::string NewName;
+  while (true) {
+    NewName = encode(Count);
+    GlobalValue *F = getNamedValue(NewName);
+    if (F == nullptr) {
+      // Reserve this entry for the new proto
+      UniquedIntrinsicNames[{id, Proto}] = Count;
+      break;
+    }
+
+    // A declaration with this name already exists. Remember it.
+    FunctionType *FT = dyn_cast<FunctionType>(F->getType()->getElementType());
+    auto UinItInserted = UniquedIntrinsicNames.insert({{id, FT}, Count});
+    if (FT == Proto) {
+      // It was a declaration for our prototype. This entry was allocated in the
+      // beginning. Update the count to match the existing declaration.
+      UinItInserted.first->second = Count;
+      break;
+    }
+
+    ++Count;
+  }
+
+  NiidItInserted.first->second = Count + 1;
+
+  return NewName;
+}
+
 // dropAllReferences() - This function causes all the subelements to "let go"
 // of all references that they are maintaining.  This allows one to 'delete' a
 // whole module at a time, even though there may be circular references... first
 // all references are dropped, and all use counts go to zero.  Then everything
 // is deleted for real.  Note that no operations are valid on an object that
 // has "dropped all references", except operator delete.
 //
 void Module::dropAllReferences() {

llvm/lib/IR/Verifier.cpp

void Verifier::visitIntrinsicCall(Intrinsic::ID ID, CallBase &Call) {
 
   // All descriptors should be absorbed by now.
   Assert(TableRef.empty(), "Intrinsic has too few arguments!", IF);
 
   // Now that we have the intrinsic ID and the actual argument types (and we
   // know they are legal for the intrinsic!) get the intrinsic name through the
   // usual means.  This allows us to verify the mangling of argument types into
   // the name.
-  const std::string ExpectedName = Intrinsic::getName(ID, ArgTys);
+  const std::string ExpectedName =
+      Intrinsic::getName(ID, ArgTys, IF->getParent(), IFTy);
   Assert(ExpectedName == IF->getName(),
          "Intrinsic name not mangled correctly for type arguments! "
          "Should be: " +
              ExpectedName,
          IF);
 
   // If the intrinsic takes MDNode arguments, verify that they are either global
   // or are local to *this* function.

llvm/lib/Linker/IRMover.cpp

GlobalValue *getLinkedToGlobal(const GlobalValue *SrcGV) {
     if (!DGV)
       return nullptr;
 
     // If we found a global with the same name in the dest module, but it has
     // internal linkage, we are really not doing any linkage here.
     if (DGV->hasLocalLinkage())
       return nullptr;
 
+    // If we found an intrinsic declaration with mismatching prototypes, we
+    // probably had a nameclash. Don't use that version.
+    if (auto *FDGV = dyn_cast<Function>(DGV))
+      if (FDGV->isIntrinsic())
+        if (auto *FSrcGV = dyn_cast<Function>(SrcGV))
+          if (FDGV->getFunctionType() != TypeMap.get(FSrcGV->getFunctionType()))
+            return nullptr;
+
     // Otherwise, we do in fact link to the destination global.
     return DGV;
   }
 
   void computeTypeMapping();
 
   Expected<Constant *> linkAppendingVarProto(GlobalVariable *DstGV,
                                              const GlobalVariable *SrcGV);
if (!ShouldLink && ForIndirectSymbol)
     DGV = nullptr;
 
   // Handle the ultra special appending linkage case first.
   assert(!DGV || SGV->hasAppendingLinkage() == DGV->hasAppendingLinkage());
   if (SGV->hasAppendingLinkage())
     return linkAppendingVarProto(cast_or_null<GlobalVariable>(DGV),
                                  cast<GlobalVariable>(SGV));
 
+  bool NeedsRenaming = false;
   GlobalValue *NewGV;
   if (DGV && !ShouldLink) {
     NewGV = DGV;
   } else {
     // If we are done linking global value bodies (i.e. we are performing
     // metadata linking), don't link in the global value due to this
     // reference, simply map it to null.
     if (DoneLinkingBodies)
       return nullptr;
 
     NewGV = copyGlobalValueProto(SGV, ShouldLink || ForIndirectSymbol);
     if (ShouldLink || !ForIndirectSymbol)
-      forceRenaming(NewGV, SGV->getName());
+      NeedsRenaming = true;
   }
 
   // Overloaded intrinsics have overloaded types names as part of their
   // names. If we renamed overloaded types we should rename the intrinsic
   // as well.
   if (Function *F = dyn_cast<Function>(NewGV))
-    if (auto Remangled = Intrinsic::remangleIntrinsicFunction(F))
+    if (auto Remangled = Intrinsic::remangleIntrinsicFunction(F)) {
+      NewGV->eraseFromParent();
       NewGV = Remangled.getValue();
+      NeedsRenaming = false;
+    }
+
+  if (NeedsRenaming)
+    forceRenaming(NewGV, SGV->getName());
 
   if (ShouldLink || ForIndirectSymbol) {
     if (const Comdat *SC = SGV->getComdat()) {
       if (auto *GO = dyn_cast<GlobalObject>(NewGV)) {
         Comdat *DC = DstM.getOrInsertComdat(SC->getName());
         DC->setSelectionKind(SC->getSelectionKind());
         GO->setComdat(DC);
       }

llvm/test/Bitcode/intrinsics-with-unnamed-types.ll

+; RUN: llvm-as -o - %s | llvm-dis -o - 2>&1 | FileCheck %s
+
+; Make sure we can assemble and disassemble IR containing intrinsics with
+; unnamed types.
+
+%1 = type opaque
+%0 = type opaque
+
+; CHECK-LABEL: @f0(
+; CHECK: %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+; CHECK: %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+; CHECK: %c3 = call %0** @llvm.ssa.copy.p0p0s_s.1(%0** %arg2)
+; CHECK: %c4 = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+
+define void @f0(%0* %arg, %1* %tmp, %1** %tmp2, %0** %arg2) {
+bb:
+  %cmp1 = icmp ne %0* %arg, null
+  %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+  %c3 = call %0** @llvm.ssa.copy.p0p0s_s.1(%0** %arg2)
+  %c4 = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  ret void
+}
+
+declare %0* @llvm.ssa.copy.p0s_s.0(%0* returned)
+
+declare %1* @llvm.ssa.copy.p0s_s.1(%1* returned)
+
+declare %0** @llvm.ssa.copy.p0p0s_s.1(%0** returned)
+
+declare %1** @llvm.ssa.copy.p0p0s_s.0(%1** returned)

llvm/test/Linker/intrinsics-with-unnamed-types.ll

+; RUN: split-file %s %t
+; RUN: llvm-as -o %t1.bc %t/f01.ll
+; RUN: llvm-as -o %t2.bc %t/f02.ll
+; RUN: llvm-link %t1.bc %t2.bc -o %t3.bc
+; RUN: llvm-dis -o - %t3.bc | FileCheck %s
+
+; Make sure we can link files with clashing intrinsic names using unnamed types.
+
+;--- f01.ll
+%1 = type opaque
+%0 = type opaque
+
+; CHECK-LABEL: @test01(
+; CHECK:       %cmp1 = icmp ne %0* %arg, null
+; CHECK-NEXT:  %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+; CHECK-NEXT:  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+; CHECK-NEXT:  %c3a = call %0** @llvm.ssa.copy.p0p0s_s.0(%0** %arg2)
+; CHECK-NEXT:  %c3b = call %0** @llvm.ssa.copy.p0p0s_s.0(%0** %arg2)
+; CHECK-NEXT:  %c4a = call %1** @llvm.ssa.copy.p0p0s_s.1(%1** %tmp2)
+; CHECK-NEXT:  %c4ba = call %1** @llvm.ssa.copy.p0p0s_s.1(%1** %tmp2)
+; CHECK-NEXT:  %c5 = call %0*** @llvm.ssa.copy.p0p0p0s_s.0(%0*** %arg3)
+; CHECK-NEXT:  %c6 = call %1*** @llvm.ssa.copy.p0p0p0s_s.1(%1*** %tmp3)
+
+define void @test01(%0* %arg, %1* %tmp, %1** %tmp2, %0** %arg2, %1*** %tmp3, %0*** %arg3) {
+bb:
+  %cmp1 = icmp ne %0* %arg, null
+  %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+  %c3a = call %0** @llvm.ssa.copy.p0p0s_s.1(%0** %arg2)
+  %c3b = call %0** @llvm.ssa.copy.p0p0s_s.1(%0** %arg2)
+  %c4a = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  %c4ba = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  %c5 = call %0*** @llvm.ssa.copy.p0p0p0s_s.1(%0*** %arg3)
+  %c6 = call %1*** @llvm.ssa.copy.p0p0p0s_s.0(%1*** %tmp3)
+  ret void
+}
+
+declare %0* @llvm.ssa.copy.p0s_s.0(%0* returned)
+
+declare %1* @llvm.ssa.copy.p0s_s.1(%1* returned)
+
+declare %0** @llvm.ssa.copy.p0p0s_s.1(%0** returned)
+
+declare %1** @llvm.ssa.copy.p0p0s_s.0(%1** returned)
+
+declare %0*** @llvm.ssa.copy.p0p0p0s_s.1(%0*** returned)
+
+declare %1*** @llvm.ssa.copy.p0p0p0s_s.0(%1*** returned)
+
+; now with recycling of previous declarations:
+; CHECK-LABEL: @test02(
+; CHECK:       %cmp1 = icmp ne %0* %arg, null
+; CHECK-NEXT:  %c4a = call %1** @llvm.ssa.copy.p0p0s_s.1(%1** %tmp2)
+; CHECK-NEXT:  %c6 = call %1*** @llvm.ssa.copy.p0p0p0s_s.1(%1*** %tmp3)
+; CHECK-NEXT:  %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+; CHECK-NEXT:  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+; CHECK-NEXT:  %c3b = call %0** @llvm.ssa.copy.p0p0s_s.0(%0** %arg2)
+; CHECK-NEXT:  %c4ba = call %1** @llvm.ssa.copy.p0p0s_s.1(%1** %tmp2)
+; CHECK-NEXT:  %c5 = call %0*** @llvm.ssa.copy.p0p0p0s_s.0(%0*** %arg3)
+
+define void @test02(%0* %arg, %1* %tmp, %1** %tmp2, %0** %arg2, %1*** %tmp3, %0*** %arg3) {
+bb:
+  %cmp1 = icmp ne %0* %arg, null
+  %c4a = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  %c6 = call %1*** @llvm.ssa.copy.p0p0p0s_s.0(%1*** %tmp3)
+  %c1 = call %0* @llvm.ssa.copy.p0s_s.0(%0* %arg)
+  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+  %c3b = call %0** @llvm.ssa.copy.p0p0s_s.1(%0** %arg2)
+  %c4ba = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  %c5 = call %0*** @llvm.ssa.copy.p0p0p0s_s.1(%0*** %arg3)
+  ret void
+}
+
+;--- f02.ll
+%1 = type opaque
+%2 = type opaque
+
+; CHECK-LABEL: @test03(
+; CHECK:      %cmp1 = icmp ne %3* %arg, null
+; CHECK-NEXT: %c1 = call %3* @llvm.ssa.copy.p0s_s.2(%3* %arg)
+; CHECK-NEXT: %c2 = call %2* @llvm.ssa.copy.p0s_s.3(%2* %tmp)
+; CHECK-NEXT: %c3 = call %3** @llvm.ssa.copy.p0p0s_s.2(%3** %arg2)
+; CHECK-NEXT: %c4 = call %2** @llvm.ssa.copy.p0p0s_s.3(%2** %tmp2)
+
+define void @test03(%1* %tmp, %2* %arg, %1** %tmp2, %2** %arg2) {
+bb:
+  %cmp1 = icmp ne %2* %arg, null
+  %c1 = call %2* @llvm.ssa.copy.p0s_s.0(%2* %arg)
+  %c2 = call %1* @llvm.ssa.copy.p0s_s.1(%1* %tmp)
+  %c3 = call %2** @llvm.ssa.copy.p0p0s_s.1(%2** %arg2)
+  %c4 = call %1** @llvm.ssa.copy.p0p0s_s.0(%1** %tmp2)
+  ret void
+}
+
+declare %2* @llvm.ssa.copy.p0s_s.0(%2* returned)
+
+declare %1* @llvm.ssa.copy.p0s_s.1(%1* returned)
+
+declare %2** @llvm.ssa.copy.p0p0s_s.1(%2** returned)
+
+declare %1** @llvm.ssa.copy.p0p0s_s.0(%1** returned)