Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emulate @weak functions on Windows and don't emit COMDATs for ELF anymore #3424

Merged
merged 2 commits into from
May 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions gen/functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include "llvm/IR/CFG.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetOptions.h"
#include "llvm/Transforms/Utils/Cloning.h"
#include <iostream>

bool isAnyMainFunction(FuncDeclaration *fd) {
Expand Down Expand Up @@ -892,6 +893,62 @@ bool eraseDummyAfterReturnBB(llvm::BasicBlock *bb) {
return false;
}

/**
* LLVM doesn't really support weak linkage for MSVC targets, it just prevents
* inlining. We can emulate it though, by conceptually renaming the defined
* function, only declaring the original function and embedding a linker
* directive in the object file, instructing the linker to fall back to the weak
* implementation if there's no strong definition.
* The object file still needs to be pulled in by the linker for the directive
* to be found.
*/
void emulateWeakAnyLinkageForMSVC(LLFunction *func, LINK linkage) {
const bool isWin32 = !global.params.is64bit;

std::string mangleBuffer;
llvm::StringRef finalMangle = func->getName();
if (finalMangle[0] == '\1') {
finalMangle = finalMangle.substr(1);
} else if (isWin32) {
// implicit underscore prefix for Win32
mangleBuffer = ("_" + finalMangle).str();
finalMangle = mangleBuffer;
}

std::string finalWeakMangle = finalMangle;
if (linkage == LINKcpp) {
assert(finalMangle.startswith("?"));
// prepend `__weak_` to first identifier
size_t offset = finalMangle.startswith("??$") ? 3 : 1;
finalWeakMangle.insert(offset, "__weak_");
} else if (linkage == LINKd) {
const size_t offset = isWin32 ? 1 : 0;
assert(finalMangle.substr(offset).startswith("_D"));
// prepend a `__weak` package
finalWeakMangle.insert(offset + 2, "6__weak");
} else {
// prepend `__weak_`
const size_t offset = isWin32 && finalMangle.startswith("_") ? 1 : 0;
finalWeakMangle.insert(offset, "__weak_");
}

const std::string linkerOption =
("/ALTERNATENAME:" + finalMangle + "=" + finalWeakMangle).str();
gIR->addLinkerOption(llvm::StringRef(linkerOption));

// work around LLVM assertion when cloning a function's debuginfos
func->setSubprogram(nullptr);

llvm::ValueToValueMapTy dummy;
auto clone = llvm::CloneFunction(func, dummy);
clone->setName("\1" + finalWeakMangle);
setLinkage({LLGlobalValue::ExternalLinkage, func->hasComdat()}, clone);

// reduce the original definition to a declaration
setLinkage({LLGlobalValue::ExternalLinkage, false}, func);
func->deleteBody();
}

} // anonymous namespace

void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
Expand Down Expand Up @@ -1274,6 +1331,11 @@ void DtoDefineFunction(FuncDeclaration *fd, bool linkageAvailableExternally) {
auto fn = gIR->module.getFunction(fd->mangleString);
gIR->dcomputetarget->addKernelMetadata(fd, fn);
}

if (func->getLinkage() == LLGlobalValue::WeakAnyLinkage &&
global.params.targetTriple->isWindowsMSVCEnvironment()) {
emulateWeakAnyLinkageForMSVC(func, fd->linkage);
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion gen/moduleinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,6 @@ llvm::GlobalVariable *genModuleInfo(Module *m) {
// Create a global symbol with the above initialiser.
LLGlobalVariable *moduleInfoSym = getIrModule(m)->moduleInfoSymbol();
b.finalize(moduleInfoSym);
setLinkage({LLGlobalValue::ExternalLinkage, supportsCOMDAT()}, moduleInfoSym);
setLinkage({LLGlobalValue::ExternalLinkage, needsCOMDAT()}, moduleInfoSym);
return moduleInfoSym;
}
2 changes: 1 addition & 1 deletion gen/pgo_ASTbased.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ void CodeGenPGO::setFuncName(llvm::StringRef Name,
// If Linkage is private, and the function is in a comdat "any" group, set
// the linkage to internal to prevent LLVM from erroring with "comdat global
// value has private linkage".
if (supportsCOMDAT() &&
if (needsCOMDAT() &&
FuncNameVar->getLinkage() == llvm::GlobalValue::PrivateLinkage) {
FuncNameVar->setLinkage(llvm::GlobalValue::InternalLinkage);
}
Expand Down
4 changes: 2 additions & 2 deletions gen/rttibuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void RTTIBuilder::push_void_array(llvm::Constant *CI, Type *valtype,
mangleToBuffer(mangle_sym, &initname);
initname.writestring(".rtti.voidarr.data");

const LinkageWithCOMDAT lwc(TYPEINFO_LINKAGE_TYPE, supportsCOMDAT());
const LinkageWithCOMDAT lwc(TYPEINFO_LINKAGE_TYPE, needsCOMDAT());

auto G = new LLGlobalVariable(gIR->module, CI->getType(), true, lwc.first, CI,
initname.peekChars());
Expand All @@ -111,7 +111,7 @@ void RTTIBuilder::push_array(llvm::Constant *CI, uint64_t dim, Type *valtype,
initname.writestring(tmpStr.c_str());
initname.writestring(".data");

const LinkageWithCOMDAT lwc(TYPEINFO_LINKAGE_TYPE, supportsCOMDAT());
const LinkageWithCOMDAT lwc(TYPEINFO_LINKAGE_TYPE, needsCOMDAT());

auto G = new LLGlobalVariable(gIR->module, CI->getType(), true, lwc.first, CI,
initname.peekChars());
Expand Down
26 changes: 13 additions & 13 deletions gen/tollvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,25 +223,25 @@ LinkageWithCOMDAT DtoLinkage(Dsymbol *sym) {
linkage = LLGlobalValue::WeakAnyLinkage;
}

return {linkage, supportsCOMDAT()};
return {linkage, needsCOMDAT()};
}

bool supportsCOMDAT() {
const auto &triple = *global.params.targetTriple;
return !(triple.isOSBinFormatMachO() ||
#if LDC_LLVM_VER >= 500
triple.isOSBinFormatWasm()
#else
triple.getArch() == llvm::Triple::wasm32 ||
triple.getArch() == llvm::Triple::wasm64
#endif
);
bool needsCOMDAT() {
/* For MSVC targets (and probably MinGW too), linkonce[_odr] and weak[_odr]
* linkages don't work and need to be emulated via COMDATs to prevent multiple
* definition errors when linking.
* Simply emit all functions in COMDATs, not just templates, for aggressive
* linker stripping (/OPT:REF and /OPT:ICF with MS linker/LLD), analogous to
* using /Gy with the MS compiler.
* https://docs.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=vs-2019
*/
return global.params.targetTriple->isOSBinFormatCOFF();
}

void setLinkage(LinkageWithCOMDAT lwc, llvm::GlobalObject *obj) {
obj->setLinkage(lwc.first);
if (lwc.second)
obj->setComdat(gIR->module.getOrInsertComdat(obj->getName()));
obj->setComdat(lwc.second ? gIR->module.getOrInsertComdat(obj->getName())
: nullptr);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I've also tried setting the COMDAT's selection kind to 'exact match' (instead of 'any'), but that only led back to the multiple definition errors.

}

void setLinkageAndVisibility(Dsymbol *sym, llvm::GlobalObject *obj) {
Expand Down
2 changes: 1 addition & 1 deletion gen/tollvm.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ LLValue *DtoDelegateEquals(TOK op, LLValue *lhs, LLValue *rhs);
typedef std::pair<llvm::GlobalValue::LinkageTypes, bool> LinkageWithCOMDAT;
LinkageWithCOMDAT DtoLinkage(Dsymbol *sym);

bool supportsCOMDAT();
bool needsCOMDAT();
void setLinkage(LinkageWithCOMDAT lwc, llvm::GlobalObject *obj);
// Sets the linkage of the specified IR global and possibly hides it, both based
// on the specified D symbol.
Expand Down
2 changes: 1 addition & 1 deletion gen/typinf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ void TypeInfoDeclaration_codegen(TypeInfoDeclaration *decl, IRState *p) {
LLVMDefineVisitor v(gvar);
decl->accept(&v);

setLinkage({TYPEINFO_LINKAGE_TYPE, supportsCOMDAT()}, gvar);
setLinkage({TYPEINFO_LINKAGE_TYPE, needsCOMDAT()}, gvar);
if (auto forStructType = forType->isTypeStruct())
setVisibility(forStructType->sym, gvar);
}
Expand Down
2 changes: 1 addition & 1 deletion ir/irclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ void IrAggr::defineInterfaceVtbl(BaseClass *b, bool new_instance,
llvm::Function *thunk = gIR->module.getFunction(thunkIRMangle);
if (!thunk) {
const LinkageWithCOMDAT lwc(LLGlobalValue::LinkOnceODRLinkage,
supportsCOMDAT());
needsCOMDAT());
const auto callee = irFunc->getLLVMCallee();
thunk = LLFunction::Create(
isaFunction(callee->getType()->getContainedType(0)), lwc.first,
Expand Down
2 changes: 1 addition & 1 deletion runtime/druntime
7 changes: 5 additions & 2 deletions tests/PGO/irbased_indirect_calls.d
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@

// REQUIRES: PGO_RT

// with LLVM 4.0, @optStrategy apparently doesn't suffice to prevent eliding the hot() call
// XFAIL: llvm400

// RUN: %ldc -O3 -fprofile-generate=%t.profraw -run %s \
// RUN: && %profdata merge %t.profraw -o %t.profdata \
// RUN: && %ldc -O3 -c -output-ll -of=%t.use.ll -fprofile-use=%t.profdata %s \
// RUN: && FileCheck %s -check-prefix=PROFUSE < %t.use.ll

import ldc.attributes : weak;
import ldc.attributes;

extern (C)
{ // simplify name mangling for simpler string matching

@weak // disable reasoning about this function
@optStrategy("none")
void hot()
{
}
Expand Down
17 changes: 10 additions & 7 deletions tests/codegen/attr_weak.d
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
// Test linking+running a program with @weak function
// Test linking+running a program with @weak functions

// RUN: %ldc -O3 %S/inputs/attr_weak_input.d -c -of=%t-dir/attr_weak_input%obj
// RUN: %ldc -O3 %t-dir/attr_weak_input%obj %s -of=%t%exe
// RUN: %ldc -O3 %s %t-dir/attr_weak_input%obj -of=%t%exe
// RUN: %t%exe


import ldc.attributes;

// Should be overridden by attr_weak_input.d (but only because its object
// file is specified before this one for the linker).
// The @weak attribute prevents the optimizer from making any assumptions
// though, so the call below is not inlined.
// should take precedence over and not conflict with weak attr_weak_input.return_two
extern(C) int return_two() {
return 123;
}

// should be overridden by strong attr_weak_input.return_seven
extern(C) @weak int return_seven() {
return 1;
return 456;
kinke marked this conversation as resolved.
Show resolved Hide resolved
}

void main() {
assert( return_two() == 123 );
assert( return_seven() == 7 );
}
2 changes: 1 addition & 1 deletion tests/codegen/attributes.d
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import ldc.attributes;
// CHECK-DAG: @{{.*}}myWeakGlobali{{\"?}} = weak
@(ldc.attributes.weak) int myWeakGlobal;

// CHECK-DAG: define{{.*}} weak {{.*}}void @{{.*}}weakFunc
// CHECK-DAG: define{{.*}} {{(weak .*void @.*_D)|(void @.*_D6__weak)}}10attributes8weakFuncFZv
@weak void weakFunc() {}

//---------------------------------------------------------------------
Expand Down
6 changes: 6 additions & 0 deletions tests/codegen/inputs/attr_weak_input.d
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
import ldc.attributes;

extern(C) @weak int return_two() {
return 2;
}

extern(C) int return_seven() {
return 7;
}