Skip to content

Commit 86e5b25

Browse files
compnerdcompnerd
committed
IRGen: introduce new IRLinkage applicator
In order to handle LinkOnceODR semantics correctly across various object formats, introduce a new helper ApplyIRLinkage. This abstracts the need to create a COMDAT group and set it on the GlobalValue. Adjust all sites where we set the IR linkage attributes to use this mechanism instead to avoid having to track down symbols not being added to a COMDAT group.
1 parent eea96a3 commit 86e5b25

File tree

6 files changed

+98
-75
lines changed

6 files changed

+98
-75
lines changed

‎include/swift/IRGen/Linking.h

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#include "swift/SIL/SILModule.h"
2525
#include "llvm/ADT/DenseMapInfo.h"
2626
#include "llvm/IR/GlobalValue.h"
27+
#include "llvm/IR/GlobalObject.h"
28+
#include "llvm/IR/Module.h"
2729

2830
namespace llvm {
2931
class Triple;
@@ -935,14 +937,43 @@ class LinkEntity {
935937
#undef LINKENTITY_SET_FIELD
936938
};
937939

940+
struct IRLinkage {
941+
llvm::GlobalValue::LinkageTypes Linkage;
942+
llvm::GlobalValue::VisibilityTypes Visibility;
943+
llvm::GlobalValue::DLLStorageClassTypes DLLStorage;
944+
};
945+
946+
class ApplyIRLinkage {
947+
IRLinkage IRL;
948+
public:
949+
ApplyIRLinkage(IRLinkage IRL) : IRL(IRL) {}
950+
void to(llvm::GlobalValue *GV) const {
951+
GV->setLinkage(IRL.Linkage);
952+
GV->setVisibility(IRL.Visibility);
953+
GV->setDLLStorageClass(IRL.DLLStorage);
954+
955+
if (IRL.Linkage == llvm::GlobalValue::LinkOnceODRLinkage ||
956+
IRL.Linkage == llvm::GlobalValue::WeakODRLinkage) {
957+
llvm::Module *M = GV->getParent();
958+
const llvm::Triple Triple(M->getTargetTriple());
959+
960+
// TODO: BFD and gold do not handle COMDATs properly
961+
if (Triple.isOSBinFormatELF())
962+
return;
963+
964+
if (Triple.supportsCOMDAT())
965+
if (llvm::GlobalObject *GO = dyn_cast<llvm::GlobalObject>(GV))
966+
GO->setComdat(M->getOrInsertComdat(GV->getName()));
967+
}
968+
}
969+
};
970+
938971
/// Encapsulated information about the linkage of an entity.
939972
class LinkInfo {
940973
LinkInfo() = default;
941974

942975
llvm::SmallString<32> Name;
943-
llvm::GlobalValue::LinkageTypes Linkage;
944-
llvm::GlobalValue::VisibilityTypes Visibility;
945-
llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass;
976+
IRLinkage IRL;
946977
ForDefinition_t ForDefinition;
947978

948979
public:
@@ -962,25 +993,19 @@ class LinkInfo {
962993
return Name.str();
963994
}
964995
llvm::GlobalValue::LinkageTypes getLinkage() const {
965-
return Linkage;
996+
return IRL.Linkage;
966997
}
967998
llvm::GlobalValue::VisibilityTypes getVisibility() const {
968-
return Visibility;
999+
return IRL.Visibility;
9691000
}
9701001
llvm::GlobalValue::DLLStorageClassTypes getDLLStorage() const {
971-
return DLLStorageClass;
1002+
return IRL.DLLStorage;
9721003
}
9731004

974-
bool isForDefinition() const {
975-
return ForDefinition;
976-
}
977-
bool isUsed() const {
978-
return ForDefinition && isUsed(Linkage, Visibility, DLLStorageClass);
979-
}
1005+
bool isForDefinition() const { return ForDefinition; }
1006+
bool isUsed() const { return ForDefinition && isUsed(IRL); }
9801007

981-
static bool isUsed(llvm::GlobalValue::LinkageTypes Linkage,
982-
llvm::GlobalValue::VisibilityTypes Visibility,
983-
llvm::GlobalValue::DLLStorageClassTypes DLLStorage);
1008+
static bool isUsed(IRLinkage IRL);
9841009
};
9851010

9861011
StringRef encodeForceLoadSymbolName(llvm::SmallVectorImpl<char> &buf,

‎lib/IRGen/GenDecl.cpp

Lines changed: 42 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -917,8 +917,10 @@ llvm::Constant *IRGenModule::getAddrOfAssociatedTypeGenericParamRef(
917917

918918
auto var = B.finishAndCreateGlobal(symbolName, Alignment(4),
919919
/*constant*/ true);
920-
var->setLinkage(llvm::GlobalValue::LinkOnceODRLinkage);
921-
var->setVisibility(llvm::GlobalValue::HiddenVisibility);
920+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
921+
llvm::GlobalValue::HiddenVisibility,
922+
llvm::GlobalValue::DefaultStorageClass})
923+
.to(var);
922924
setTrueConstGlobal(var);
923925
return var;
924926
}
@@ -1348,16 +1350,14 @@ void IRGenModule::emitVTableStubs() {
13481350
}
13491351
}
13501352

1351-
static std::tuple<llvm::GlobalValue::LinkageTypes,
1352-
llvm::GlobalValue::VisibilityTypes,
1353-
llvm::GlobalValue::DLLStorageClassTypes>
1353+
static IRLinkage
13541354
getIRLinkage(const UniversalLinkageInfo &info, SILLinkage linkage,
13551355
ForDefinition_t isDefinition,
13561356
bool isWeakImported) {
13571357
#define RESULT(LINKAGE, VISIBILITY, DLL_STORAGE) \
1358-
std::make_tuple(llvm::GlobalValue::LINKAGE##Linkage, \
1359-
llvm::GlobalValue::VISIBILITY##Visibility, \
1360-
llvm::GlobalValue::DLL_STORAGE##StorageClass)
1358+
IRLinkage{llvm::GlobalValue::LINKAGE##Linkage, \
1359+
llvm::GlobalValue::VISIBILITY##Visibility, \
1360+
llvm::GlobalValue::DLL_STORAGE##StorageClass}
13611361

13621362
// Use protected visibility for public symbols we define on ELF. ld.so
13631363
// doesn't support relative relocations at load time, which interferes with
@@ -1375,8 +1375,8 @@ getIRLinkage(const UniversalLinkageInfo &info, SILLinkage linkage,
13751375

13761376
switch (linkage) {
13771377
case SILLinkage::Public:
1378-
return std::make_tuple(llvm::GlobalValue::ExternalLinkage,
1379-
PublicDefinitionVisibility, ExportedStorage);
1378+
return {llvm::GlobalValue::ExternalLinkage, PublicDefinitionVisibility,
1379+
ExportedStorage};
13801380

13811381
case SILLinkage::Shared:
13821382
case SILLinkage::SharedExternal:
@@ -1394,34 +1394,25 @@ getIRLinkage(const UniversalLinkageInfo &info, SILLinkage linkage,
13941394
auto visibility = info.shouldAllPrivateDeclsBeVisibleFromOtherFiles()
13951395
? llvm::GlobalValue::HiddenVisibility
13961396
: llvm::GlobalValue::DefaultVisibility;
1397-
return std::make_tuple(linkage, visibility,
1398-
llvm::GlobalValue::DefaultStorageClass);
1397+
return {linkage, visibility, llvm::GlobalValue::DefaultStorageClass};
13991398
}
14001399

14011400
case SILLinkage::PublicExternal: {
1402-
if (isDefinition) {
1403-
return std::make_tuple(llvm::GlobalValue::AvailableExternallyLinkage,
1404-
llvm::GlobalValue::DefaultVisibility,
1405-
llvm::GlobalValue::DefaultStorageClass);
1406-
}
1401+
if (isDefinition)
1402+
return RESULT(AvailableExternally, Default, Default);
14071403

14081404
auto linkage = isWeakImported ? llvm::GlobalValue::ExternalWeakLinkage
14091405
: llvm::GlobalValue::ExternalLinkage;
1410-
return std::make_tuple(linkage, llvm::GlobalValue::DefaultVisibility,
1411-
ImportedStorage);
1406+
return {linkage, llvm::GlobalValue::DefaultVisibility, ImportedStorage};
14121407
}
14131408

14141409
case SILLinkage::HiddenExternal:
14151410
case SILLinkage::PrivateExternal:
1416-
if (isDefinition) {
1417-
return std::make_tuple(llvm::GlobalValue::AvailableExternallyLinkage,
1418-
llvm::GlobalValue::HiddenVisibility,
1419-
llvm::GlobalValue::DefaultStorageClass);
1420-
}
1411+
if (isDefinition)
1412+
return RESULT(AvailableExternally, Hidden, Default);
14211413

1422-
return std::make_tuple(llvm::GlobalValue::ExternalLinkage,
1423-
llvm::GlobalValue::DefaultVisibility,
1424-
ImportedStorage);
1414+
return {llvm::GlobalValue::ExternalLinkage,
1415+
llvm::GlobalValue::DefaultVisibility, ImportedStorage};
14251416

14261417
}
14271418

@@ -1436,22 +1427,18 @@ void irgen::updateLinkageForDefinition(IRGenModule &IGM,
14361427
// TODO: there are probably cases where we can avoid redoing the
14371428
// entire linkage computation.
14381429
UniversalLinkageInfo linkInfo(IGM);
1439-
auto linkage =
1430+
auto IRL =
14401431
getIRLinkage(linkInfo, entity.getLinkage(ForDefinition),
14411432
ForDefinition, entity.isWeakImported(IGM.getSwiftModule()));
1442-
global->setLinkage(std::get<0>(linkage));
1443-
global->setVisibility(std::get<1>(linkage));
1444-
global->setDLLStorageClass(std::get<2>(linkage));
1433+
ApplyIRLinkage(IRL).to(global);
14451434

14461435
// Everything externally visible is considered used in Swift.
14471436
// That mostly means we need to be good at not marking things external.
14481437
//
14491438
// Exclude "main", because it should naturally be used, and because adding it
14501439
// to llvm.used leaves a dangling use when the REPL attempts to discard
14511440
// intermediate mains.
1452-
if (LinkInfo::isUsed(std::get<0>(linkage), std::get<1>(linkage),
1453-
std::get<2>(linkage)) &&
1454-
global->getName() != SWIFT_ENTRY_POINT_FUNCTION)
1441+
if (LinkInfo::isUsed(IRL) && global->getName() != SWIFT_ENTRY_POINT_FUNCTION)
14551442
IGM.addUsedGlobal(global);
14561443
}
14571444

@@ -1477,9 +1464,8 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo,
14771464
ForDefinition_t(swiftModule->isStdlibModule() || isDefinition);
14781465

14791466
entity.mangle(result.Name);
1480-
std::tie(result.Linkage, result.Visibility, result.DLLStorageClass) =
1481-
getIRLinkage(linkInfo, entity.getLinkage(isStdlibOrDefinition),
1482-
isDefinition, entity.isWeakImported(swiftModule));
1467+
result.IRL = getIRLinkage(linkInfo, entity.getLinkage(isStdlibOrDefinition),
1468+
isDefinition, entity.isWeakImported(swiftModule));
14831469
result.ForDefinition = isDefinition;
14841470
return result;
14851471
}
@@ -1490,8 +1476,7 @@ LinkInfo LinkInfo::get(const UniversalLinkageInfo &linkInfo, StringRef name,
14901476
LinkInfo result;
14911477

14921478
result.Name += name;
1493-
std::tie(result.Linkage, result.Visibility, result.DLLStorageClass) =
1494-
getIRLinkage(linkInfo, linkage, isDefinition, isWeakImported);
1479+
result.IRL = getIRLinkage(linkInfo, linkage, isDefinition, isWeakImported);
14951480
result.ForDefinition = isDefinition;
14961481
return result;
14971482
}
@@ -1523,6 +1508,7 @@ llvm::Function *irgen::createFunction(IRGenModule &IGM,
15231508

15241509
llvm::Function *fn =
15251510
llvm::Function::Create(signature.getType(), linkInfo.getLinkage(), name);
1511+
// TODO(compnerd) apply COMDAT to definitions
15261512
fn->setVisibility(linkInfo.getVisibility());
15271513
fn->setDLLStorageClass(linkInfo.getDLLStorage());
15281514
fn->setCallingConv(signature.getCallingConv());
@@ -1531,7 +1517,7 @@ llvm::Function *irgen::createFunction(IRGenModule &IGM,
15311517
IGM.Module.getFunctionList().insert(insertBefore->getIterator(), fn);
15321518
} else {
15331519
IGM.Module.getFunctionList().push_back(fn);
1534-
}
1520+
}
15351521

15361522
llvm::AttrBuilder initialAttrs;
15371523
IGM.constructInitialFnAttributes(initialAttrs, FuncOptMode);
@@ -1556,16 +1542,14 @@ llvm::Function *irgen::createFunction(IRGenModule &IGM,
15561542
return fn;
15571543
}
15581544

1559-
bool LinkInfo::isUsed(llvm::GlobalValue::LinkageTypes Linkage,
1560-
llvm::GlobalValue::VisibilityTypes Visibility,
1561-
llvm::GlobalValue::DLLStorageClassTypes DLLStorage) {
1545+
bool LinkInfo::isUsed(IRLinkage IRL) {
15621546
// Everything externally visible is considered used in Swift.
15631547
// That mostly means we need to be good at not marking things external.
1564-
return Linkage == llvm::GlobalValue::ExternalLinkage &&
1565-
(Visibility == llvm::GlobalValue::DefaultVisibility ||
1566-
Visibility == llvm::GlobalValue::ProtectedVisibility) &&
1567-
(DLLStorage == llvm::GlobalValue::DefaultStorageClass ||
1568-
DLLStorage == llvm::GlobalValue::DLLExportStorageClass);
1548+
return IRL.Linkage == llvm::GlobalValue::ExternalLinkage &&
1549+
(IRL.Visibility == llvm::GlobalValue::DefaultVisibility ||
1550+
IRL.Visibility == llvm::GlobalValue::ProtectedVisibility) &&
1551+
(IRL.DLLStorage == llvm::GlobalValue::DefaultStorageClass ||
1552+
IRL.DLLStorage == llvm::GlobalValue::DLLExportStorageClass);
15691553
}
15701554

15711555
/// Get or create an LLVM global variable with these linkage rules.
@@ -1591,8 +1575,10 @@ llvm::GlobalVariable *swift::irgen::createVariable(
15911575
auto var = new llvm::GlobalVariable(IGM.Module, storageType,
15921576
/*constant*/ false, linkInfo.getLinkage(),
15931577
/*initializer*/ nullptr, name);
1594-
var->setVisibility(linkInfo.getVisibility());
1595-
var->setDLLStorageClass(linkInfo.getDLLStorage());
1578+
ApplyIRLinkage({linkInfo.getLinkage(),
1579+
linkInfo.getVisibility(),
1580+
linkInfo.getDLLStorage()})
1581+
.to(var);
15961582
var->setAlignment(alignment.getValue());
15971583

15981584
// Everything externally visible is considered used in Swift.
@@ -2868,8 +2854,8 @@ llvm::GlobalValue *IRGenModule::defineAlias(LinkEntity entity,
28682854
auto *alias = llvm::GlobalAlias::create(
28692855
ptrTy->getElementType(), ptrTy->getAddressSpace(), link.getLinkage(),
28702856
link.getName(), definition, &Module);
2871-
alias->setVisibility(link.getVisibility());
2872-
alias->setDLLStorageClass(link.getDLLStorage());
2857+
ApplyIRLinkage({link.getLinkage(), link.getVisibility(), link.getDLLStorage()})
2858+
.to(alias);
28732859

28742860
if (link.isUsed()) {
28752861
addUsedGlobal(alias);
@@ -3797,9 +3783,10 @@ static llvm::Function *shouldDefineHelper(IRGenModule &IGM,
37973783
if (!def) return nullptr;
37983784
if (!def->empty()) return nullptr;
37993785

3800-
def->setLinkage(llvm::Function::LinkOnceODRLinkage);
3801-
def->setVisibility(llvm::Function::HiddenVisibility);
3802-
def->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
3786+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
3787+
llvm::GlobalValue::HiddenVisibility,
3788+
llvm::GlobalValue::DefaultStorageClass})
3789+
.to(def);
38033790
def->setDoesNotThrow();
38043791
def->setCallingConv(IGM.DefaultCC);
38053792
if (setIsNoInline)

‎lib/IRGen/GenEnum.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1473,7 +1473,10 @@ namespace {
14731473
auto func =
14741474
llvm::Function::Create(consumeTy, llvm::GlobalValue::LinkOnceODRLinkage,
14751475
llvm::StringRef(name), IGM.getModule());
1476-
func->setVisibility(llvm::GlobalValue::HiddenVisibility);
1476+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
1477+
llvm::GlobalValue::HiddenVisibility,
1478+
llvm::GlobalValue::DefaultStorageClass})
1479+
.to(func);
14771480
func->setAttributes(IGM.constructInitialAttributes());
14781481
func->setDoesNotThrow();
14791482
func->setCallingConv(IGM.DefaultCC);

‎lib/IRGen/GenObjC.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -963,8 +963,10 @@ static llvm::Constant *findSwiftAsObjCThunk(IRGenModule &IGM, SILDeclRef ref) {
963963
SILFunction *SILFn = IGM.getSILModule().lookUpFunction(ref);
964964
assert(SILFn && "no IR function for swift-as-objc thunk");
965965
auto fn = IGM.getAddrOfSILFunction(SILFn, NotForDefinition);
966-
fn->setVisibility(llvm::GlobalValue::DefaultVisibility);
967-
fn->setLinkage(llvm::GlobalValue::InternalLinkage);
966+
ApplyIRLinkage({llvm::GlobalValue::InternalLinkage,
967+
llvm::GlobalValue::DefaultVisibility,
968+
llvm::GlobalValue::DefaultStorageClass})
969+
.to(fn);
968970
fn->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
969971

970972
return llvm::ConstantExpr::getBitCast(fn, IGM.Int8PtrTy);

‎lib/IRGen/GenReflection.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,10 @@ void IRGenModule::emitReflectionMetadataVersion() {
971971
llvm::GlobalValue::LinkOnceODRLinkage,
972972
Init,
973973
"__swift_reflection_version");
974-
Version->setVisibility(llvm::GlobalValue::HiddenVisibility);
974+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
975+
llvm::GlobalValue::HiddenVisibility,
976+
llvm::GlobalValue::DefaultStorageClass})
977+
.to(Version);
975978
addUsedGlobal(Version);
976979
}
977980

‎lib/IRGen/MetadataRequest.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,10 @@ llvm::Constant *IRGenModule::getAddrOfStringForTypeRef(
317317
llvm::GlobalValue::LinkOnceODRLinkage,
318318
nullptr,
319319
symbolName);
320-
var->setVisibility(llvm::GlobalValue::HiddenVisibility);
320+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
321+
llvm::GlobalValue::HiddenVisibility,
322+
llvm::GlobalValue::DefaultStorageClass})
323+
.to(var);
321324
var->setAlignment(2);
322325
setTrueConstGlobal(var);
323326
var->setSection(getReflectionTypeRefSectionName());

0 commit comments

Comments
 (0)