<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 5, 2016 at 11:30 AM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span class=""><blockquote type="cite"><div>On May 5, 2016, at 11:25 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Wed, May 4, 2016 at 10:14 PM, Mehdi Amini via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: mehdi_amini<br>Date: Thu May  5 00:14:24 2016<br>New Revision: 268607<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=268607&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=268607&view=rev</a><br>Log:<br>LTOCodeGenerator: add linkonce(_odr) to "llvm.compiler.used" when present in "MustPreserve" set<br><br>If the linker requested to preserve a linkonce function, we should<br>honor this even if we drop all uses.<br>We explicitely avoid turning them into weak_odr (unlike the first<br>version of this patch in r267644), because the codegen can be<br>different on Darwin: because of `llvm::canBeOmittedFromSymbolTable()`<br>we may emit the symbol as weak_def_can_be_hidden instead of<br>weak_definition.<br></blockquote><div><br></div><div>Do we only do this on Darwin then, and otherwise turn it into weak_odr everywhere else?</div></div></div></blockquote><div><br></div></span><div>To be sure I understand: you mean we would use llvm.compiler.used on Darwin, and the weak_odr conversion otherwise?</div></div></div></blockquote><div><br></div><div>Yes - this, of course, isn't my wheelhouse, so there may be lots of good reasons not to do this, I'm just curious.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>I'm not fan of special-casing, do you see a drawback with the llvm.compiler.used approach?<br></div></div></div></blockquote><div><br></div><div>Don't know enough about it - sounds like it might lead to a different linkage in the generated object, which could have an impact on linking? or maybe it all comes out the same in the end?<br><br>But it seems like Darwin is the special case here - so it seems strange to do the same odd things to non-Darwin platforms in the name of consistency. But, again, I don't have lots of context here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div><div>Ideally, I'd prefer to get ride of the "weak_def_can_be_hidden" thing we do at the MC level. It is not clear to me why it is not expressed at the IR level, but I haven't had time to give it more thoughts.<br></div></div></div></blockquote><div><br></div><div>*nod*<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div></div><span class="HOEnZb"><font color="#888888"><div><br></div><div><br></div><div>-- </div><div>Mehdi</div></font></span><div><div class="h5"><div><br></div><div><br></div><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>From: Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>><br><br>Modified:<br>   <span> </span>llvm/trunk/lib/LTO/LTOCodeGenerator.cpp<br>   <span> </span>llvm/trunk/test/tools/lto/hide-linkonce-odr.ll<br><br>Modified: llvm/trunk/lib/LTO/LTOCodeGenerator.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=268607&r1=268606&r2=268607&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=268607&r1=268606&r2=268607&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)<br>+++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Thu May  5 00:14:24 2016<br>@@ -348,8 +348,73 @@ std::unique_ptr<TargetMachine> LTOCodeGe<br>                                 <span> </span>RelocModel, CodeModel::Default, CGOptLevel));<br> }<br><br>+// If a linkonce global is present in the MustPreserveSymbols, we need to make<br>+// sure we honor this. To force the compiler to not drop it, we add it to the<br>+// "llvm.compiler.used" global.<br>+static void preserveDiscardableGVs(<br>+    Module &TheModule,<br>+    llvm::function_ref<bool(const GlobalValue &)> mustPreserveGV) {<br>+  SetVector<Constant *> UsedValuesSet;<br>+  if (GlobalVariable *LLVMUsed =<br>+          TheModule.getGlobalVariable("llvm.compiler.used")) {<br>+    ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());<br>+    for (auto &V : Inits->operands())<br>+      UsedValuesSet.insert(cast<Constant>(&V));<br>+    LLVMUsed->eraseFromParent();<br>+  }<br>+  llvm::Type *i8PTy = llvm::Type::getInt8PtrTy(TheModule.getContext());<br>+  auto mayPreserveGlobal = [&](GlobalValue &GV) {<br>+    if (!GV.isDiscardableIfUnused() || GV.isDeclaration())<br>+      return;<br>+    if (!mustPreserveGV(GV))<br>+      return;<br>+    assert(!GV.hasAvailableExternallyLinkage() && !GV.hasInternalLinkage());<br>+    UsedValuesSet.insert(ConstantExpr::getBitCast(&GV, i8PTy));<br>+  };<br>+  for (auto &GV : TheModule)<br>+    mayPreserveGlobal(GV);<br>+  for (auto &GV : TheModule.globals())<br>+    mayPreserveGlobal(GV);<br>+  for (auto &GV : TheModule.aliases())<br>+    mayPreserveGlobal(GV);<br>+<br>+  if (UsedValuesSet.empty())<br>+    return;<br>+<br>+  llvm::ArrayType *ATy = llvm::ArrayType::get(i8PTy, UsedValuesSet.size());<br>+  auto *LLVMUsed = new llvm::GlobalVariable(<br>+      TheModule, ATy, false, llvm::GlobalValue::AppendingLinkage,<br>+      llvm::ConstantArray::get(ATy, UsedValuesSet.getArrayRef()),<br>+      "llvm.compiler.used");<br>+  LLVMUsed->setSection("llvm.metadata");<br>+}<br>+<br> void LTOCodeGenerator::applyScopeRestrictions() {<br>-  if (ScopeRestrictionsDone || !ShouldInternalize)<br>+  if (ScopeRestrictionsDone)<br>+    return;<br>+<br>+  // Declare a callback for the internalize pass that will ask for every<br>+  // candidate GlobalValue if it can be internalized or not.<br>+  SmallString<64> MangledName;<br>+  auto mustPreserveGV = [&](const GlobalValue &GV) -> bool {<br>+    // Unnamed globals can't be mangled, but they can't be preserved either.<br>+    if (!GV.hasName())<br>+      return false;<br>+<br>+    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled<br>+    // with the linker supplied name, which on Darwin includes a leading<br>+    // underscore.<br>+    MangledName.clear();<br>+    MangledName.reserve(GV.getName().size() + 1);<br>+    Mangler::getNameWithPrefix(MangledName, GV.getName(),<br>+                               MergedModule->getDataLayout());<br>+    return MustPreserveSymbols.count(MangledName);<br>+  };<br>+<br>+  // Preserve linkonce value on linker request<br>+  preserveDiscardableGVs(*MergedModule, mustPreserveGV);<br>+<br>+  if (!ShouldInternalize)<br>     return;<br><br>   if (ShouldRestoreGlobalsLinkage) {<br>@@ -373,22 +438,7 @@ void LTOCodeGenerator::applyScopeRestric<br>   // symbols referenced from asm<br>   UpdateCompilerUsed(*MergedModule, *TargetMach, AsmUndefinedRefs);<br><br>-  // Declare a callback for the internalize pass that will ask for every<br>-  // candidate GlobalValue if it can be internalized or not.<br>-  Mangler Mangler;<br>-  SmallString<64> MangledName;<br>-  auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {<br>-    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled<br>-    // with the linker supplied name, which on Darwin includes a leading<br>-    // underscore.<br>-    MangledName.clear();<br>-    MangledName.reserve(GV.getName().size() + 1);<br>-    Mangler::getNameWithPrefix(MangledName, GV.getName(),<br>-                               MergedModule->getDataLayout());<br>-    return MustPreserveSymbols.count(MangledName);<br>-  };<br>-<br>-  internalizeModule(*MergedModule, MustPreserveGV);<br>+  internalizeModule(*MergedModule, mustPreserveGV);<br><br>   ScopeRestrictionsDone = true;<br> }<br><br>Modified: llvm/trunk/test/tools/lto/hide-linkonce-odr.ll<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/lto/hide-linkonce-odr.ll?rev=268607&r1=268606&r2=268607&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/lto/hide-linkonce-odr.ll?rev=268607&r1=268606&r2=268607&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/tools/lto/hide-linkonce-odr.ll (original)<br>+++ llvm/trunk/test/tools/lto/hide-linkonce-odr.ll Thu May  5 00:14:24 2016<br>@@ -1,21 +1,28 @@<br> ; RUN: llvm-as %s -o %t.o<br>-; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup<br>+; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup -exported_symbol _c -exported_symbol _b  -exported_symbol _GlobLinkonce<br><br> ; RUN: llvm-dis %t.dylib.lto.opt.bc -o - | FileCheck --check-prefix=IR %s<br>-; check that @a is still a linkonce_odr definition<br>-; IR: define linkonce_odr void @a()<br>+; check that @a is no longer a linkonce_odr definition<br>+; IR-NOT: define linkonce_odr void @a()<br>+; check that @b is appended in llvm.used<br>+; IR: @llvm.compiler.used = appending global [2 x i8*] [i8* bitcast ([1 x i8*]* @GlobLinkonce to i8*), i8* bitcast (void ()* @b to i8*)], section "llvm.metadata"<br><br> ; RUN: llvm-nm %t.dylib | FileCheck --check-prefix=NM %s<br>-; check that the linker can hide @a but not @b<br>+; check that the linker can hide @a but not @b, nor @GlobLinkonce<br>+; NM: 0000000000000f48 S _GlobLinkonce<br> ; NM: 0000000000000f10 t _a<br> ; NM: 0000000000000f20 T _b<br> ; NM: 0000000000000f00 T _c<br><br>+<br> target triple = "x86_64-apple-macosx10.10.0"<br><br> declare void @external()<br><br>+@GlobLinkonce = linkonce_odr unnamed_addr constant [1 x i8*] [i8* null], align 8<br>+<br> define linkonce_odr void @a() noinline {<br>+  %use_of_GlobLinkonce = load [1 x i8*], [1 x i8*] *@GlobLinkonce<br>   call void @external()<br>   ret void<br> }<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></div></div></div><br></div></blockquote></div><br></div></div>