Ask the module for its the identified types.

When lazy reading a module, the types used in a function will not be visible to
a TypeFinder until the body is read.

This patch fixes that by asking the module for its identified struct types.
If a materializer is present, the module asks it. If not, it uses a TypeFinder.

This fixes pr21374.

I will be the first to say that this is ugly, but it was the best I could find.

Some of the options I looked at:

* Asking the LLVMContext. This could be made to work for gold, but not currently
  for ld64. ld64 will load multiple modules into a single context before merging
  them. This causes us to see types from future merges. Unfortunately,
  MappedTypes is not just a cache when it comes to opaque types. Once the
  mapping has been made, we have to remember it for as long as the key may
  be used. This would mean moving MappedTypes to the Linker class and having
  to drop the Linker::LinkModules static methods, which are visible from C.

* Adding an option to ignore function bodies in the TypeFinder. This would
  fix the PR by picking the worst result. It would work, but unfortunately
  we are currently quite dependent on the upfront type merging. I will
  try to reduce our dependency, but it is not clear that we will be able
  to get rid of it for now.

The only clean solution I could think of is making the Module own the types.
This would have other advantages, but it is a much bigger change. I will
propose it, but it is nice to have this fixed while that is discussed.

With the gold plugin, this patch takes the number of types in the LTO clang
binary from 52817 to 49669.

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

diff --git a/include/llvm/IR/GVMaterializer.h b/include/llvm/IR/GVMaterializer.h
index a7d68eccba57fb94f6b1ab91bd7ac03bafedc02b..6f57dc2a98a6abf2722cb51e2515dac3a165b3bf 100644 (file)
--- a/include/llvm/IR/GVMaterializer.h
+++ b/include/llvm/IR/GVMaterializer.h
@@ -19,11 +19,13 @@
 #define LLVM_IR_GVMATERIALIZER_H
 
 #include <system_error>
+#include <vector>
 
 namespace llvm {
 class Function;
 class GlobalValue;
 class Module;
+class StructType;
 
 class GVMaterializer {
 protected:
@@ -50,6 +52,8 @@ public:
   /// Make sure the entire Module has been completely read.
   ///
   virtual std::error_code MaterializeModule(Module *M) = 0;
+
+  virtual std::vector<StructType *> getIdentifiedStructTypes() const = 0;
 };
 
 } // End llvm namespace

diff --git a/include/llvm/IR/Module.h b/include/llvm/IR/Module.h
index 7fff80ae24b7b289ea9eff67be255ecab663bddd..30d3d7c48335d58322e857fe689e3e24ee43b19d 100644 (file)
--- a/include/llvm/IR/Module.h
+++ b/include/llvm/IR/Module.h
@@ -327,6 +327,8 @@ public:
   /// name.
   StructType *getTypeByName(StringRef Name) const;
 
+  std::vector<StructType *> getIdentifiedStructTypes() const;
+
 /// @}
 /// @name Function Accessors
 /// @{

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index 5dab73991de9064b8007e7c290790f2f181a2a60..25efa609a192f6d6867fc4b0fa213b30aed28ff9 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -487,7 +487,20 @@ Type *BitcodeReader::getTypeByID(unsigned ID) {
 
   // If we have a forward reference, the only possible case is when it is to a
   // named struct.  Just create a placeholder for now.
-  return TypeList[ID] = StructType::create(Context);
+  return TypeList[ID] = createIdentifiedStructType(Context);
+}
+
+StructType *BitcodeReader::createIdentifiedStructType(LLVMContext &Context,
+                                                      StringRef Name) {
+  auto *Ret = StructType::create(Context, Name);
+  IdentifiedStructTypes.push_back(Ret);
+  return Ret;
+}
+
+StructType *BitcodeReader::createIdentifiedStructType(LLVMContext &Context) {
+  auto *Ret = StructType::create(Context);
+  IdentifiedStructTypes.push_back(Ret);
+  return Ret;
 }
 
 
@@ -922,7 +935,7 @@ std::error_code BitcodeReader::ParseTypeTableBody() {
         Res->setName(TypeName);
         TypeList[NumRecords] = nullptr;
       } else  // Otherwise, create a new struct.
-        Res = StructType::create(Context, TypeName);
+        Res = createIdentifiedStructType(Context, TypeName);
       TypeName.clear();
 
       SmallVector<Type*, 8> EltTys;
@@ -951,7 +964,7 @@ std::error_code BitcodeReader::ParseTypeTableBody() {
         Res->setName(TypeName);
         TypeList[NumRecords] = nullptr;
       } else  // Otherwise, create a new struct with no body.
-        Res = StructType::create(Context, TypeName);
+        Res = createIdentifiedStructType(Context, TypeName);
       TypeName.clear();
       ResultTy = Res;
       break;
@@ -3416,6 +3429,10 @@ std::error_code BitcodeReader::MaterializeModule(Module *M) {
   return std::error_code();
 }
 
+std::vector<StructType *> BitcodeReader::getIdentifiedStructTypes() const {
+  return IdentifiedStructTypes;
+}
+
 std::error_code BitcodeReader::InitStream() {
   if (LazyStreamer)
     return InitLazyStream();

diff --git a/lib/Bitcode/Reader/BitcodeReader.h b/lib/Bitcode/Reader/BitcodeReader.h
index 070300ac3e0e2ae73c06958717e441ac278e1ccc..10f870b59b809b7bd262a4cff941f4ee4ebd7859 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.h
+++ b/lib/Bitcode/Reader/BitcodeReader.h
@@ -227,6 +227,7 @@ public:
   bool isDematerializable(const GlobalValue *GV) const override;
   std::error_code materialize(GlobalValue *GV) override;
   std::error_code MaterializeModule(Module *M) override;
+  std::vector<StructType *> getIdentifiedStructTypes() const override;
   void Dematerialize(GlobalValue *GV) override;
 
   /// @brief Main interface to parsing a bitcode buffer.
@@ -240,6 +241,10 @@ public:
   static uint64_t decodeSignRotatedValue(uint64_t V);
 
 private:
+  std::vector<StructType *> IdentifiedStructTypes;
+  StructType *createIdentifiedStructType(LLVMContext &Context, StringRef Name);
+  StructType *createIdentifiedStructType(LLVMContext &Context);
+
   Type *getTypeByID(unsigned ID);
   Value *getFnValueByID(unsigned ID, Type *Ty) {
     if (Ty && Ty->isMetadataTy())

diff --git a/lib/IR/Module.cpp b/lib/IR/Module.cpp
index 14e534b8b1bbafea8be71058d116673cda8e5762..065c66e149179fb3ab1d30750b1e3eb00b072760 100644 (file)
--- a/lib/IR/Module.cpp
+++ b/lib/IR/Module.cpp
@@ -23,6 +23,7 @@
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LeakDetector.h"
+#include "llvm/IR/TypeFinder.h"
 #include "llvm/Support/Dwarf.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/RandomNumberGenerator.h"
@@ -425,6 +426,19 @@ std::error_code Module::materializeAllPermanently() {
 // Other module related stuff.
 //
 
+std::vector<StructType *> Module::getIdentifiedStructTypes() const {
+  // If we have a materializer, it is possible that some unread function
+  // uses a type that is currently not visible to a TypeFinder, so ask
+  // the materializer which types it created.
+  if (Materializer)
+    return Materializer->getIdentifiedStructTypes();
+
+  std::vector<StructType *> Ret;
+  TypeFinder SrcStructTypes;
+  SrcStructTypes.run(*this, true);
+  Ret.assign(SrcStructTypes.begin(), SrcStructTypes.end());
+  return Ret;
+}
 
 // dropAllReferences() - This function causes all the subelements to "let go"
 // of all references that they are maintaining.  This allows one to 'delete' a

diff --git a/lib/Linker/LinkModules.cpp b/lib/Linker/LinkModules.cpp
index ebb790201fd24ac9e5254ef9c98dd3519eeba7b0..ecfb9431974efb1f3decc5b0d2683f246e16a6f1 100644 (file)
--- a/lib/Linker/LinkModules.cpp
+++ b/lib/Linker/LinkModules.cpp
@@ -791,10 +791,8 @@ void ModuleLinker::computeTypeMapping() {
   // At this point, the destination module may have a type "%foo = { i32 }" for
   // example.  When the source module got loaded into the same LLVMContext, if
   // it had the same type, it would have been renamed to "%foo.42 = { i32 }".
-  TypeFinder SrcStructTypes;
-  SrcStructTypes.run(*SrcM, true);
-
-  for (StructType *ST : SrcStructTypes) {
+  std::vector<StructType *> Types = SrcM->getIdentifiedStructTypes();
+  for (StructType *ST : Types) {
     if (!ST->hasName())
       continue;
 

diff --git a/test/Linker/Inputs/pr21374.ll b/test/Linker/Inputs/pr21374.ll
new file mode 100644 (file)
index 0000000..fcddeaf
--- /dev/null
+++ b/test/Linker/Inputs/pr21374.ll
@@ -0,0 +1,4 @@
+%foo = type { i8* }
+define void @g(%foo* %x) {
+  ret void
+}

diff --git a/test/Linker/pr21374.ll b/test/Linker/pr21374.ll
new file mode 100644 (file)
index 0000000..d777971
--- /dev/null
+++ b/test/Linker/pr21374.ll
@@ -0,0 +1,20 @@
+; RUN: llvm-link -S -o - %p/pr21374.ll %p/Inputs/pr21374.ll | FileCheck %s
+; RUN: llvm-link -S -o - %p/Inputs/pr21374.ll %p/pr21374.ll | FileCheck %s
+
+; RUN: llvm-as -o %t1.bc %p/pr21374.ll
+; RUN: llvm-as -o %t2.bc %p/Inputs/pr21374.ll
+
+; RUN: llvm-link -S -o - %t1.bc %t2.bc | FileCheck %s
+; RUN: llvm-link -S -o - %t2.bc %t1.bc | FileCheck %s
+
+; Test that we get the same result with or without lazy loading.
+
+; CHECK: %foo = type { i8* }
+; CHECK-DAG: bitcast i32* null to %foo*
+; CHECK-DAG: define void @g(%foo* %x)
+
+%foo = type { i8* }
+define void @f() {
+  bitcast i32* null to %foo*
+  ret void
+}