Add support for comdats to the gold plugin.
authorRafael Espindola <rafael.espindola@gmail.com>
Fri, 22 Aug 2014 23:26:10 +0000 (23:26 +0000)
committerRafael Espindola <rafael.espindola@gmail.com>
Fri, 22 Aug 2014 23:26:10 +0000 (23:26 +0000)
There are two parts to this. First, the plugin needs to tell gold the comdat by
setting comdat_key.

What gets things a bit more complicated is that gold only seems
symbols. In particular, if A is an alias to B, it only sees the symbols
A and B. It can then ask us to keep symbol A but drop symbol B. What
we have to do instead is to create an internal version of B and make A
an alias to that.

At some point some of this logic should be moved to lib/Linker so that
we don't map a Constant to an internal version just to have lib/Linker
map that again to the destination module.

The reason for implementing this in tools/gold for now is simplicity.
With it in place it should be possible to update clang to use comdats
for constructors and destructors on ELF without breaking the LTO
bootstrap. Once that is done I intend to come back and improve the
interface lib/Linker exposes.

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

test/tools/gold/Inputs/comdat.ll [new file with mode: 0644]
test/tools/gold/bad-alias.ll [new file with mode: 0644]
test/tools/gold/comdat.ll [new file with mode: 0644]
tools/gold/gold-plugin.cpp

diff --git a/test/tools/gold/Inputs/comdat.ll b/test/tools/gold/Inputs/comdat.ll
new file mode 100644 (file)
index 0000000..b9a639a
--- /dev/null
@@ -0,0 +1,19 @@
+$c2 = comdat any
+
+@v1 = weak_odr global i32 41, comdat $c2
+define weak_odr i32 @f1() comdat $c2 {
+bb20:
+  br label %bb21
+bb21:
+  ret i32 41
+}
+
+@r21 = global i32* @v1
+@r22 = global i32()* @f1
+
+@a21 = alias i32* @v1
+@a22 = alias bitcast (i32* @v1 to i16*)
+
+@a23 = alias i32()* @f1
+@a24 = alias bitcast (i32()* @f1 to i16*)
+@a25 = alias i16* @a24
diff --git a/test/tools/gold/bad-alias.ll b/test/tools/gold/bad-alias.ll
new file mode 100644 (file)
index 0000000..e0fc788
--- /dev/null
@@ -0,0 +1,13 @@
+; RUN: llvm-as %s -o %t.o
+
+; RUN: not ld -plugin %llvmshlibdir/LLVMgold.so \
+; RUN:    --plugin-opt=emit-llvm \
+; RUN:    -shared %t.o -o %t2.o 2>&1 | FileCheck %s
+
+; CHECK: Unable to determine comdat of alias!
+
+@g1 = global i32 1
+@g2 = global i32 2
+
+@a = alias inttoptr(i32 sub (i32 ptrtoint (i32* @g1 to i32),
+                             i32 ptrtoint (i32* @g2 to i32)) to i32*)
diff --git a/test/tools/gold/comdat.ll b/test/tools/gold/comdat.ll
new file mode 100644 (file)
index 0000000..cb8599e
--- /dev/null
@@ -0,0 +1,64 @@
+; RUN: llvm-as %s -o %t.o
+; RUN: llvm-as %p/Inputs/comdat.ll -o %t2.o
+; RUN: ld -shared -o %t3.o -plugin %llvmshlibdir/LLVMgold.so %t.o %t2.o \
+; RUN:  -plugin-opt=emit-llvm
+; RUN: llvm-dis %t3.o -o - | FileCheck %s
+
+$c1 = comdat any
+
+@v1 = weak_odr global i32 42, comdat $c1
+define weak_odr i32 @f1() comdat $c1 {
+bb10:
+  br label %bb11
+bb11:
+  ret i32 42
+}
+
+@r11 = global i32* @v1
+@r12 = global i32 ()* @f1
+
+@a11 = alias i32* @v1
+@a12 = alias bitcast (i32* @v1 to i16*)
+
+@a13 = alias i32 ()* @f1
+@a14 = alias bitcast (i32 ()* @f1 to i16*)
+@a15 = alias i16* @a14
+
+; CHECK: $c1 = comdat any
+; CHECK: $c2 = comdat any
+
+; CHECK: @v1 = weak_odr global i32 42, comdat $c1
+
+; CHECK: @r11 = global i32* @v1{{$}}
+; CHECK: @r12 = global i32 ()* @f1{{$}}
+
+; CHECK: @r21 = global i32* @v1{{$}}
+; CHECK: @r22 = global i32 ()* @f1{{$}}
+
+; CHECK: @v11 = internal global i32 41, comdat $c2
+
+; CHECK: @a11 = alias i32* @v1{{$}}
+; CHECK: @a12 = alias bitcast (i32* @v1 to i16*)
+
+; CHECK: @a13 = alias i32 ()* @f1{{$}}
+; CHECK: @a14 = alias bitcast (i32 ()* @f1 to i16*)
+
+; CHECK: @a21 = alias i32* @v11{{$}}
+; CHECK: @a22 = alias bitcast (i32* @v11 to i16*)
+
+; CHECK: @a23 = alias i32 ()* @f12{{$}}
+; CHECK: @a24 = alias bitcast (i32 ()* @f12 to i16*)
+
+; CHECK:      define weak_odr i32 @f1() comdat $c1 {
+; CHECK-NEXT: bb10:
+; CHECK-NEXT:   br label %bb11{{$}}
+; CHECK:      bb11:
+; CHECK-NEXT:   ret i32 42
+; CHECK-NEXT: }
+
+; CHECK:      define internal i32 @f12() comdat $c2 {
+; CHECK-NEXT: bb20:
+; CHECK-NEXT:   br label %bb21
+; CHECK:      bb21:
+; CHECK-NEXT:   ret i32 41
+; CHECK-NEXT: }
index 25a21b7a1562441cb471189aa28185eb6156a3fa..8ca2628e85aa1cc38f5661f165a88a3279b138e4 100644 (file)
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Config/config.h" // plugin-api.h requires HAVE_STDINT_H
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/CodeGen/Analysis.h"
@@ -34,6 +35,7 @@
 #include "llvm/Transforms/IPO/PassManagerBuilder.h"
 #include "llvm/Transforms/Utils/GlobalStatus.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
+#include "llvm/Transforms/Utils/ValueMapper.h"
 #include <list>
 #include <plugin-api.h>
 #include <system_error>
@@ -255,6 +257,12 @@ ld_plugin_status onload(ld_plugin_tv *tv) {
   return LDPS_OK;
 }
 
+static const GlobalObject *getBaseObject(const GlobalValue &GV) {
+  if (auto *GA = dyn_cast<GlobalAlias>(&GV))
+    return GA->getBaseObject();
+  return cast<GlobalObject>(&GV);
+}
+
 /// Called by gold to see whether this file is one that our plugin can handle.
 /// We'll try to open it and register all the symbols with add_symbol if
 /// possible.
@@ -362,8 +370,16 @@ static ld_plugin_status claim_file_hook(const ld_plugin_input_file *file,
 
     sym.size = 0;
     sym.comdat_key = nullptr;
-    if (GV && (GV->hasWeakLinkage() || GV->hasLinkOnceLinkage()))
-      sym.comdat_key = sym.name;
+    if (GV) {
+      const GlobalObject *Base = getBaseObject(*GV);
+      if (!Base)
+        message(LDPL_FATAL, "Unable to determine comdat of alias!");
+      const Comdat *C = Base->getComdat();
+      if (C)
+        sym.comdat_key = strdup(C->getName().str().c_str());
+      else if (Base->hasWeakLinkage() || Base->hasLinkOnceLinkage())
+        sym.comdat_key = strdup(sym.name);
+    }
 
     sym.resolution = LDPR_UNKNOWN;
   }
@@ -378,9 +394,13 @@ static ld_plugin_status claim_file_hook(const ld_plugin_input_file *file,
   return LDPS_OK;
 }
 
-static void keepGlobalValue(GlobalValue &GV) {
+static void keepGlobalValue(GlobalValue &GV,
+                            std::vector<GlobalAlias *> &KeptAliases) {
   assert(!GV.hasLocalLinkage());
 
+  if (auto *GA = dyn_cast<GlobalAlias>(&GV))
+    KeptAliases.push_back(GA);
+
   switch (GV.getLinkage()) {
   default:
     break;
@@ -415,12 +435,15 @@ static void internalize(GlobalValue &GV) {
 static void drop(GlobalValue &GV) {
   if (auto *F = dyn_cast<Function>(&GV)) {
     F->deleteBody();
+    F->setComdat(nullptr); // Should deleteBody do this?
     return;
   }
 
   if (auto *Var = dyn_cast<GlobalVariable>(&GV)) {
     Var->setInitializer(nullptr);
-    Var->setLinkage(GlobalValue::ExternalLinkage);
+    Var->setLinkage(
+        GlobalValue::ExternalLinkage); // Should setInitializer do this?
+    Var->setComdat(nullptr); // and this?
     return;
   }
 
@@ -460,6 +483,55 @@ static const char *getResolutionName(ld_plugin_symbol_resolution R) {
   }
 }
 
+static GlobalObject *makeInternalReplacement(GlobalObject *GO) {
+  Module *M = GO->getParent();
+  GlobalObject *Ret;
+  if (auto *F = dyn_cast<Function>(GO)) {
+    auto *NewF = Function::Create(
+        F->getFunctionType(), GlobalValue::InternalLinkage, F->getName(), M);
+    NewF->getBasicBlockList().splice(NewF->end(), F->getBasicBlockList());
+    Ret = NewF;
+    F->deleteBody();
+  } else {
+    auto *Var = cast<GlobalVariable>(GO);
+    Ret = new GlobalVariable(
+        *M, Var->getType()->getElementType(), Var->isConstant(),
+        GlobalValue::InternalLinkage, Var->getInitializer(), Var->getName(),
+        nullptr, Var->getThreadLocalMode(), Var->getType()->getAddressSpace(),
+        Var->isExternallyInitialized());
+    Var->setInitializer(nullptr);
+  }
+  Ret->copyAttributesFrom(GO);
+  Ret->setComdat(GO->getComdat());
+
+  return Ret;
+}
+
+namespace {
+class LocalValueMaterializer : public ValueMaterializer {
+  DenseSet<GlobalValue *> &Dropped;
+
+public:
+  LocalValueMaterializer(DenseSet<GlobalValue *> &Dropped) : Dropped(Dropped) {}
+  Value *materializeValueFor(Value *V) override;
+};
+}
+
+Value *LocalValueMaterializer::materializeValueFor(Value *V) {
+  auto *GV = dyn_cast<GlobalValue>(V);
+  if (!GV)
+    return nullptr;
+  if (!Dropped.count(GV))
+    return nullptr;
+  assert(!isa<GlobalAlias>(GV) && "Found alias point to weak alias.");
+  return makeInternalReplacement(cast<GlobalObject>(GV));
+}
+
+static Constant *mapConstantToLocalCopy(Constant *C, ValueToValueMapTy &VM,
+                                        LocalValueMaterializer *Materializer) {
+  return MapValue(C, VM, RF_IgnoreMissingEntries, nullptr, Materializer);
+}
+
 static std::unique_ptr<Module>
 getModuleForFile(LLVMContext &Context, claimed_file &F, raw_fd_ostream *ApiFile,
                  StringSet<> &Internalize, StringSet<> &Maybe) {
@@ -492,7 +564,8 @@ getModuleForFile(LLVMContext &Context, claimed_file &F, raw_fd_ostream *ApiFile,
   SmallPtrSet<GlobalValue *, 8> Used;
   collectUsedGlobalVariables(*M, Used, /*CompilerUsed*/ false);
 
-  std::vector<GlobalValue *> Drop;
+  DenseSet<GlobalValue *> Drop;
+  std::vector<GlobalAlias *> KeptAliases;
   for (ld_plugin_symbol &Sym : F.syms) {
     ld_plugin_symbol_resolution Resolution =
         (ld_plugin_symbol_resolution)Sym.resolution;
@@ -516,23 +589,23 @@ getModuleForFile(LLVMContext &Context, claimed_file &F, raw_fd_ostream *ApiFile,
       break;
 
     case LDPR_PREVAILING_DEF_IRONLY: {
+      keepGlobalValue(*GV, KeptAliases);
       if (!Used.count(GV)) {
         // Since we use the regular lib/Linker, we cannot just internalize GV
         // now or it will not be copied to the merged module. Instead we force
         // it to be copied and then internalize it.
-        keepGlobalValue(*GV);
         Internalize.insert(Sym.name);
       }
       break;
     }
 
     case LDPR_PREVAILING_DEF:
-      keepGlobalValue(*GV);
+      keepGlobalValue(*GV, KeptAliases);
       break;
 
     case LDPR_PREEMPTED_REG:
     case LDPR_PREEMPTED_IR:
-      Drop.push_back(GV);
+      Drop.insert(GV);
       break;
 
     case LDPR_PREVAILING_DEF_IRONLY_EXP: {
@@ -542,25 +615,37 @@ getModuleForFile(LLVMContext &Context, claimed_file &F, raw_fd_ostream *ApiFile,
       // copy will be LDPR_PREEMPTED_IR.
       if (GV->hasLinkOnceODRLinkage())
         Maybe.insert(Sym.name);
-      keepGlobalValue(*GV);
+      keepGlobalValue(*GV, KeptAliases);
       break;
     }
     }
 
     free(Sym.name);
+    free(Sym.comdat_key);
     Sym.name = nullptr;
     Sym.comdat_key = nullptr;
   }
 
-  if (!Drop.empty()) {
+  if (!Drop.empty())
     // This is horrible. Given how lazy loading is implemented, dropping
     // the body while there is a materializer present doesn't work, the
     // linker will just read the body back.
     M->materializeAllPermanently();
-    for (auto *GV : Drop)
-      drop(*GV);
+
+  ValueToValueMapTy VM;
+  LocalValueMaterializer Materializer(Drop);
+  for (GlobalAlias *GA : KeptAliases) {
+    // Gold told us to keep GA. It is possible that a GV usied in the aliasee
+    // expression is being dropped. If that is the case, that GV must be copied.
+    Constant *Aliasee = GA->getAliasee();
+    Constant *Replacement = mapConstantToLocalCopy(Aliasee, VM, &Materializer);
+    if (Aliasee != Replacement)
+      GA->setAliasee(Replacement);
   }
 
+  for (auto *GV : Drop)
+    drop(*GV);
+
   return M;
 }