[LTO] Handles commons in monolithic LTO

The gold-plugin was doing this internally, now the API is handling
commons correctly based on the given resolution.

Differential Revision: https://reviews.llvm.org/D23739

llvm-svn: 279417

Author: joker-eph

llvm/include/llvm/LTO/LTO.h

@@ -306,6 +306,11 @@ class LTO {
 
   struct RegularLTOState {
     RegularLTOState(unsigned ParallelCodeGenParallelismLevel, Config &Conf);
+    struct CommonResolution {
+      uint64_t Size = 0;
+      unsigned Align = 0;
+    };
+    std::map<std::string, CommonResolution> Commons;
 
     unsigned ParallelCodeGenParallelismLevel;
     LTOLLVMContext Ctx;

llvm/lib/LTO/LTO.cpp

@@ -292,6 +292,14 @@ Error LTO::addRegularLTO(std::unique_ptr<InputFile> Input,
         break;
       }
     }
+    // Common resolution: collect the maximum size/alignment.
+    // FIXME: right now we ignore the prevailing information, it is not clear
+    // what is the "right" behavior here.
+    if (Sym.getFlags() & object::BasicSymbolRef::SF_Common) {
+      auto &CommonRes = RegularLTO.Commons[Sym.getIRName()];
+      CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
+      CommonRes.Align = std::max(CommonRes.Align, Sym.getCommonAlignment());
+    }
 
     // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
   }
@@ -361,6 +369,31 @@ Error LTO::run(AddOutputFn AddOutput) {
 }
 
 Error LTO::runRegularLTO(AddOutputFn AddOutput) {
+  // Make sure commons have the right size/alignment: we kept the largest from
+  // all the prevailing when adding the inputs, and we apply it here.
+  for (auto &I : RegularLTO.Commons) {
+    ArrayType *Ty =
+        ArrayType::get(Type::getInt8Ty(RegularLTO.Ctx), I.second.Size);
+    GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
+    if (OldGV && OldGV->getType()->getElementType() == Ty) {
+      // Don't create a new global if the type is already correct, just make
+      // sure the alignment is correct.
+      OldGV->setAlignment(I.second.Align);
+      continue;
+    }
+    auto *GV = new GlobalVariable(*RegularLTO.CombinedModule, Ty, false,
+                                  GlobalValue::CommonLinkage,
+                                  ConstantAggregateZero::get(Ty), "");
+    GV->setAlignment(I.second.Align);
+    if (OldGV) {
+      OldGV->replaceAllUsesWith(ConstantExpr::getBitCast(GV, OldGV->getType()));
+      GV->takeName(OldGV);
+      OldGV->eraseFromParent();
+    } else {
+      GV->setName(I.first);
+    }
+  }
+
   if (Conf.PreOptModuleHook &&
       !Conf.PreOptModuleHook(0, *RegularLTO.CombinedModule))
     return Error();

llvm/test/tools/llvm-lto2/Inputs/common.ll

@@ -0,0 +1,7 @@
+target triple = "x86_64-apple-macosx10.11.0"
+
+@v = common global i16 0, align 4
+
+define i16 *@bar() {
+ ret i16 *@v
+}

llvm/test/tools/llvm-lto2/common.ll

@@ -0,0 +1,65 @@
+; RUN: llvm-as < %s > %t1.bc
+; RUN: llvm-as < %p/Inputs/common.ll > %t2.bc
+
+; Test that the common merging (size + alignment) is properly handled
+
+; Client marked the "large with little alignment" one as prevailing
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,x \
+; RUN:  -r %t2.bc,v,px \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s
+
+; Same as before, but reversing the order of the inputs
+; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,x \
+; RUN:  -r %t2.bc,v,px \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s
+
+
+; Client marked the "small with large alignment" one as prevailing
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,px \
+; RUN:  -r %t2.bc,v,x \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck %s
+
+; Same as before, but reversing the order of the inputs
+; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,px \
+; RUN:  -r %t2.bc,v,x \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck  %s
+
+
+; Client didn't mark any as prevailing, we keep the first one we see as "external"
+; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,x \
+; RUN:  -r %t2.bc,v,x \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck  %s
+
+; Same as before, but reversing the order of the inputs
+; RUN: llvm-lto2 %t2.bc %t1.bc -o %t.o -save-temps \
+; RUN:  -r %t1.bc,v,x \
+; RUN:  -r %t2.bc,v,x \
+; RUN:  -r %t1.bc,foo,px \
+; RUN:  -r %t2.bc,bar,px
+; RUN: llvm-dis < %t.o.0.0.preopt.bc | FileCheck  %s
+
+target triple = "x86_64-apple-macosx10.11.0"
+
+@v = common global i8 0, align 8
+
+
+; CHECK: @v = common global [2 x i8] zeroinitializer, align 8
+
+define i8 *@foo() {
+ ret i8 *@v
+}

llvm/tools/gold/gold-plugin.cpp

@@ -89,13 +89,6 @@ struct ResolutionInfo {
   bool DefaultVisibility = true;
 };
 
-struct CommonResolution {
-  bool Prevailing = false;
-  bool VisibleToRegularObj = false;
-  uint64_t Size = 0;
-  unsigned Align = 0;
-};
-
 }
 
 static ld_plugin_add_symbols add_symbols = nullptr;
@@ -109,7 +102,6 @@ static std::string output_name = "";
 static std::list<claimed_file> Modules;
 static DenseMap<int, void *> FDToLeaderHandle;
 static StringMap<ResolutionInfo> ResInfo;
-static std::map<std::string, CommonResolution> Commons;
 static std::vector<std::string> Cleanup;
 static llvm::TargetOptions TargetOpts;
 static size_t MaxTasks;
@@ -572,12 +564,10 @@ static void addModule(LTO &Lto, claimed_file &F, const void *View) {
             toString(ObjOrErr.takeError()).c_str());
 
   InputFile &Obj = **ObjOrErr;
-  bool HasThinLTOSummary =
-      hasGlobalValueSummary(Obj.getMemoryBufferRef(), diagnosticHandler);
 
   unsigned SymNum = 0;
   std::vector<SymbolResolution> Resols(F.syms.size());
-  for (auto &ObjSym : Obj.symbols()) {
+  for (LLVM_ATTRIBUTE_UNUSED auto &ObjSym : Obj.symbols()) {
     ld_plugin_symbol &Sym = F.syms[SymNum];
     SymbolResolution &R = Resols[SymNum];
     ++SymNum;
@@ -619,21 +609,6 @@ static void addModule(LTO &Lto, claimed_file &F, const void *View) {
         (IsExecutable || !Res.DefaultVisibility))
       R.FinalDefinitionInLinkageUnit = true;
 
-    if ((ObjSym.getFlags() & object::BasicSymbolRef::SF_Common) &&
-        !HasThinLTOSummary) {
-      // We ignore gold's resolution for common symbols. A common symbol with
-      // the correct size and alignment is added to the module by the pre-opt
-      // module hook if any common symbol prevailed.
-      CommonResolution &CommonRes = Commons[ObjSym.getIRName()];
-      if (R.Prevailing) {
-        CommonRes.Prevailing = true;
-        CommonRes.VisibleToRegularObj = R.VisibleToRegularObj;
-      }
-      CommonRes.Size = std::max(CommonRes.Size, ObjSym.getCommonSize());
-      CommonRes.Align = std::max(CommonRes.Align, ObjSym.getCommonAlignment());
-      R.Prevailing = false;
-    }
-
     freeSymName(Sym);
   }
 
@@ -668,32 +643,6 @@ static void getOutputFileName(SmallString<128> InFilename, bool TempOutFile,
   }
 }
 
-/// Add all required common symbols to M, which is expected to be the first
-/// combined module.
-static void addCommons(Module &M) {
-  for (auto &I : Commons) {
-    if (!I.second.Prevailing)
-      continue;
-    ArrayType *Ty =
-        ArrayType::get(Type::getInt8Ty(M.getContext()), I.second.Size);
-    GlobalVariable *OldGV = M.getNamedGlobal(I.first);
-    auto *GV = new GlobalVariable(M, Ty, false, GlobalValue::CommonLinkage,
-                                  ConstantAggregateZero::get(Ty), "");
-    GV->setAlignment(I.second.Align);
-    if (OldGV) {
-      OldGV->replaceAllUsesWith(ConstantExpr::getBitCast(GV, OldGV->getType()));
-      GV->takeName(OldGV);
-      OldGV->eraseFromParent();
-    } else {
-      GV->setName(I.first);
-    }
-    // We may only internalize commons if there is a single LTO task because
-    // other native object files may require the common.
-    if (MaxTasks == 1 && !I.second.VisibleToRegularObj)
-      GV->setLinkage(GlobalValue::InternalLinkage);
-  }
-}
-
 static CodeGenOpt::Level getCGOptLevel() {
   switch (options::OptLevel) {
   case 0:
@@ -773,12 +722,6 @@ static std::unique_ptr<LTO> createLTO() {
 
   Conf.DiagHandler = diagnosticHandler;
 
-  Conf.PreOptModuleHook = [](size_t Task, Module &M) {
-    if (Task == 0)
-      addCommons(M);
-    return true;
-  };
-
   switch (options::TheOutputType) {
   case options::OT_NORMAL:
     break;