Remangle intrinsics names when types are renamed

This is a resubmittion of previously reverted rL273568.

This is a fix for the problem mentioned in "LTO and intrinsics mangling" llvm-dev mail thread:
http://lists.llvm.org/pipermail/llvm-dev/2016-April/098387.html

Reviewers: mehdi_amini, reames

Differential Revision: http://reviews.llvm.org/D19373


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@273686 91177308-0d34-0410-b5e6-96231b3b80d8

8 files changed, 169 insertions, 2 deletions

diff --git a/include/llvm/IR/Intrinsics.h b/include/llvm/IR/Intrinsics.h
index f7573aa42a21..7a87c2167710 100644
--- a/include/llvm/IR/Intrinsics.h
+++ b/include/llvm/IR/Intrinsics.h
@@ -18,6 +18,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include <string>
 
 namespace llvm {
@@ -149,6 +150,10 @@ namespace Intrinsic {
   /// This method returns true on error.
   bool matchIntrinsicVarArg(bool isVarArg, ArrayRef<IITDescriptor> &Infos);
 
+  // Checks if the intrinsic name matches with its signature and if not
+  // returns the declaration with the same signature and remangled name.
+  llvm::Optional<Function*> remangleIntrinsicFunction(Function *F);
+
 } // End Intrinsic namespace
 
 } // End llvm namespace
diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp
index 691625209e4f..bf4934c4f739 100644
--- a/lib/AsmParser/LLParser.cpp
+++ b/lib/AsmParser/LLParser.cpp
@@ -18,12 +18,14 @@
 #include "llvm/AsmParser/SlotMapping.h"
 #include "llvm/IR/AutoUpgrade.h"
 #include "llvm/IR/CallingConv.h"
+#include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Operator.h"
@@ -210,6 +212,17 @@ bool LLParser::ValidateEndOfModule() {
   for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; )
     UpgradeCallsToIntrinsic(&*FI++); // must be post-increment, as we remove
 
+  // Some types could be renamed during loading if several modules are
+  // loaded in the same LLVMContext (LTO scenario). In this case we should
+  // remangle intrinsics names as well.
+  for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ) {
+    Function *F = &*FI++;
+    if (auto Remangled = Intrinsic::remangleIntrinsicFunction(F)) {
+      F->replaceAllUsesWith(Remangled.getValue());
+      F->eraseFromParent();
+    }
+  }
+
   UpgradeDebugInfo(*M);
 
   UpgradeModuleFlags(*M);
diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index a9c509809cbf..cdb6a1f30bf6 100644
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Bitcode/LLVMBitCodes.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/AutoUpgrade.h"
+#include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -230,8 +231,10 @@ class BitcodeReader : public GVMaterializer {
 
   // When intrinsic functions are encountered which require upgrading they are
   // stored here with their replacement function.
-  typedef DenseMap<Function*, Function*> UpgradedIntrinsicMap;
-  UpgradedIntrinsicMap UpgradedIntrinsics;
+  typedef DenseMap<Function*, Function*> UpdatedIntrinsicMap;
+  UpdatedIntrinsicMap UpgradedIntrinsics;
+  // Intrinsics which were remangled because of types rename
+  UpdatedIntrinsicMap RemangledIntrinsics;
 
   // Map the bitcode's custom MDKind ID to the Module's MDKind ID.
   DenseMap<unsigned, unsigned> MDKindMap;
@@ -3425,6 +3428,11 @@ std::error_code BitcodeReader::globalCleanup() {
     Function *NewFn;
     if (UpgradeIntrinsicFunction(&F, NewFn))
       UpgradedIntrinsics[&F] = NewFn;
+    else if (auto Remangled = Intrinsic::remangleIntrinsicFunction(&F))
+      // Some types could be renamed during loading if several modules are
+      // loaded in the same LLVMContext (LTO scenario). In this case we should
+      // remangle intrinsics names as well.
+      RemangledIntrinsics[&F] = Remangled.getValue();
   }
 
   // Look for global variables which need to be renamed.
@@ -5601,6 +5609,13 @@ std::error_code BitcodeReader::materialize(GlobalValue *GV) {
     }
   }
 
+  // Update calls to the remangled intrinsics
+  for (auto &I : RemangledIntrinsics)
+    for (auto UI = I.first->materialized_user_begin(), UE = I.first->user_end();
+         UI != UE;)
+      // Don't expect any other users than call sites
+      CallSite(*UI++).setCalledFunction(I.second);
+
   // Finish fn->subprogram upgrade for materialized functions.
   if (DISubprogram *SP = FunctionsWithSPs.lookup(F))
     F->setSubprogram(SP);
@@ -5654,6 +5669,12 @@ std::error_code BitcodeReader::materializeModule() {
     I.first->eraseFromParent();
   }
   UpgradedIntrinsics.clear();
+  // Do the same for remangled intrinsics
+  for (auto &I : RemangledIntrinsics) {
+    I.first->replaceAllUsesWith(I.second);
+    I.first->eraseFromParent();
+  }
+  RemangledIntrinsics.clear();
 
   UpgradeDebugInfo(*TheModule);
 
diff --git a/lib/IR/Function.cpp b/lib/IR/Function.cpp
index 29ed68a2c5ed..0da9b98258c3 100644
--- a/lib/IR/Function.cpp
+++ b/lib/IR/Function.cpp
@@ -1080,6 +1080,40 @@ Intrinsic::matchIntrinsicVarArg(bool isVarArg,
   return true;
 }
 
+Optional<Function*> Intrinsic::remangleIntrinsicFunction(Function *F) {
+  Intrinsic::ID ID = F->getIntrinsicID();
+  if (!ID)
+    return None;
+
+  FunctionType *FTy = F->getFunctionType();
+  // Accumulate an array of overloaded types for the given intrinsic
+  SmallVector<Type *, 4> ArgTys;
+  {
+    SmallVector<Intrinsic::IITDescriptor, 8> Table;
+    getIntrinsicInfoTableEntries(ID, Table);
+    ArrayRef<Intrinsic::IITDescriptor> TableRef = Table;
+
+    // If we encounter any problems matching the signature with the descriptor
+    // just give up remangling. It's up to verifier to report the discrepancy.
+    if (Intrinsic::matchIntrinsicType(FTy->getReturnType(), TableRef, ArgTys))
+      return None;
+    for (auto Ty : FTy->params())
+      if (Intrinsic::matchIntrinsicType(Ty, TableRef, ArgTys))
+        return None;
+    if (Intrinsic::matchIntrinsicVarArg(FTy->isVarArg(), TableRef))
+      return None;
+  }
+
+  StringRef Name = F->getName();
+  if (Name == Intrinsic::getName(ID, ArgTys))
+    return None;
+
+  auto NewDecl = Intrinsic::getDeclaration(F->getParent(), ID, ArgTys);
+  NewDecl->setCallingConv(F->getCallingConv());
+  assert(NewDecl->getFunctionType() == FTy && "Shouldn't change the signature");
+  return NewDecl;
+}
+
 /// hasAddressTaken - returns true if there are any uses of this function
 /// other than direct calls or invokes to it.
 bool Function::hasAddressTaken(const User* *PutOffender) const {
diff --git a/lib/Linker/IRMover.cpp b/lib/Linker/IRMover.cpp
index 7d09b7be868f..c82fc8b3dff2 100644
--- a/lib/Linker/IRMover.cpp
+++ b/lib/Linker/IRMover.cpp
@@ -16,6 +16,7 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/GVMaterializer.h"
+#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/TypeFinder.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Transforms/Utils/Cloning.h"
@@ -901,6 +902,14 @@ Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
     if (ShouldLink || !ForAlias)
       forceRenaming(NewGV, SGV->getName());
   }
+
+  // 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))
+      NewGV = Remangled.getValue();
+
   if (ShouldLink || ForAlias) {
     if (const Comdat *SC = SGV->getComdat()) {
       if (auto *GO = dyn_cast<GlobalObject>(NewGV)) {
diff --git a/test/LTO/X86/Inputs/remangle_intrinsics.ll b/test/LTO/X86/Inputs/remangle_intrinsics.ll
new file mode 100644
index 000000000000..75f6fd19c0d2
--- /dev/null
+++ b/test/LTO/X86/Inputs/remangle_intrinsics.ll
@@ -0,0 +1,8 @@
+%struct.rtx_def = type { i16, i16 }
+
+define void @bar(%struct.rtx_def* %a, i8 %b, i32 %c) {
+  call void  @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def* %a, i8 %b, i32 %c, i32 4, i1 true)
+  ret void
+}
+
+declare void @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def*, i8, i32, i32, i1)
diff --git a/test/LTO/X86/remangle_intrinsics.ll b/test/LTO/X86/remangle_intrinsics.ll
new file mode 100644
index 000000000000..b044a393dfff
--- /dev/null
+++ b/test/LTO/X86/remangle_intrinsics.ll
@@ -0,0 +1,23 @@
+; RUN: llvm-as < %s > %t1
+; RUN: llvm-as < %p/Inputs/remangle_intrinsics.ll > %t2
+; RUN: llvm-lto %t1 %t2 | FileCheck %s
+
+; We have "struct.rtx_def" type in both modules being LTOed. Both modules use
+; an overloaded intrinsic which has this type in its signature/name. When
+; modules are loaded one of the types is renamed to "struct.rtx_def.0".
+; The intrinsic which uses this type should be remangled/renamed as well.
+; If we didn't do that verifier would complain.
+
+; CHECK: Wrote native object file
+
+%struct.rtx_def = type { i16 }
+
+define void @foo(%struct.rtx_def* %a, i8 %b, i32 %c) {
+  call void  @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def* %a, i8 %b, i32 %c, i32 4, i1 true)
+  ret void
+}
+
+declare void @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def*, i8, i32, i32, i1)
+
+; Check that remangling code doesn't fail on an intrinsic with wrong signature
+declare void @llvm.memset.i64(i8* nocapture, i8, i64, i32) nounwind
\ No newline at end of file
diff --git a/unittests/Linker/LinkModulesTest.cpp b/unittests/Linker/LinkModulesTest.cpp
index 10a89f39869a..92c483278be9 100644
--- a/unittests/Linker/LinkModulesTest.cpp
+++ b/unittests/Linker/LinkModulesTest.cpp
@@ -306,4 +306,58 @@ TEST_F(LinkModuleTest, MoveDistinctMDs) {
   EXPECT_EQ(M3, M4->getOperand(0));
 }
 
+TEST_F(LinkModuleTest, RemangleIntrinsics) {
+  LLVMContext C;
+  SMDiagnostic Err;
+
+  // We load two modules inside the same context C. In both modules there is a
+  // "struct.rtx_def" type. In the module loaded the second (Bar) this type will
+  // be renamed to "struct.rtx_def.0". Check that the intrinsics which have this
+  // type in the signature are properly remangled.
+  const char *FooStr =
+    "%struct.rtx_def = type { i16 }\n"
+    "define void @foo(%struct.rtx_def* %a, i8 %b, i32 %c) {\n"
+    "  call void  @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def* %a, i8 %b, i32 %c, i32 4, i1 true)\n"
+    "  ret void\n"
+    "}\n"
+    "declare void @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def*, i8, i32, i32, i1)\n";
+
+  const char *BarStr =
+    "%struct.rtx_def = type { i16 }\n"
+    "define void @bar(%struct.rtx_def* %a, i8 %b, i32 %c) {\n"
+    "  call void  @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def* %a, i8 %b, i32 %c, i32 4, i1 true)\n"
+    "  ret void\n"
+    "}\n"
+    "declare void @llvm.memset.p0struct.rtx_def.i32(%struct.rtx_def*, i8, i32, i32, i1)\n";
+
+  std::unique_ptr<Module> Foo = parseAssemblyString(FooStr, Err, C);
+  assert(Foo);
+  ASSERT_TRUE(Foo.get());
+  // Foo is loaded first, so the type and the intrinsic have theis original
+  // names.
+  ASSERT_TRUE(Foo->getFunction("llvm.memset.p0struct.rtx_def.i32"));
+  ASSERT_FALSE(Foo->getFunction("llvm.memset.p0struct.rtx_def.0.i32"));
+
+  std::unique_ptr<Module> Bar = parseAssemblyString(BarStr, Err, C);
+  assert(Bar);
+  ASSERT_TRUE(Bar.get());
+  // Bar is loaded after Foo, so the type is renamed to struct.rtx_def.0. Check
+  // that the intrinsic is also renamed.
+  ASSERT_FALSE(Bar->getFunction("llvm.memset.p0struct.rtx_def.i32"));
+  ASSERT_TRUE(Bar->getFunction("llvm.memset.p0struct.rtx_def.0.i32"));
+
+  // Link two modules together.
+  auto Dst = llvm::make_unique<Module>("Linked", C);
+  ASSERT_TRUE(Dst.get());
+  Ctx.setDiagnosticHandler(expectNoDiags);
+  bool Failed = Linker::linkModules(*Foo, std::move(Bar));
+  ASSERT_FALSE(Failed);
+
+  // "struct.rtx_def" from Foo and "struct.rtx_def.0" from Bar are isomorphic
+  // types, so they must be uniquified by linker. Check that they use the same
+  // intrinsic definition.
+  Function *F = Foo->getFunction("llvm.memset.p0struct.rtx_def.i32");
+  ASSERT_EQ(F->getNumUses(), (unsigned)2);
+}
+
 } // end anonymous namespace