Skip to content

Commit 24b86d8

Browse files
compnerdcompnerd
authored
Merge pull request #20059 from compnerd/comdat-helpers
IRGen: ensure that helper functions are COMDAT'ed
2 parents e272ece + 86e5b25 commit 24b86d8

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;
@@ -938,14 +940,43 @@ class LinkEntity {
938940
#undef LINKENTITY_SET_FIELD
939941
};
940942

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

945978
llvm::SmallString<32> Name;
946-
llvm::GlobalValue::LinkageTypes Linkage;
947-
llvm::GlobalValue::VisibilityTypes Visibility;
948-
llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass;
979+
IRLinkage IRL;
949980
ForDefinition_t ForDefinition;
950981

951982
public:
@@ -965,25 +996,19 @@ class LinkInfo {
965996
return Name.str();
966997
}
967998
llvm::GlobalValue::LinkageTypes getLinkage() const {
968-
return Linkage;
999+
return IRL.Linkage;
9691000
}
9701001
llvm::GlobalValue::VisibilityTypes getVisibility() const {
971-
return Visibility;
1002+
return IRL.Visibility;
9721003
}
9731004
llvm::GlobalValue::DLLStorageClassTypes getDLLStorage() const {
974-
return DLLStorageClass;
1005+
return IRL.DLLStorage;
9751006
}
9761007

977-
bool isForDefinition() const {
978-
return ForDefinition;
979-
}
980-
bool isUsed() const {
981-
return ForDefinition && isUsed(Linkage, Visibility, DLLStorageClass);
982-
}
1008+
bool isForDefinition() const { return ForDefinition; }
1009+
bool isUsed() const { return ForDefinition && isUsed(IRL); }
9831010

984-
static bool isUsed(llvm::GlobalValue::LinkageTypes Linkage,
985-
llvm::GlobalValue::VisibilityTypes Visibility,
986-
llvm::GlobalValue::DLLStorageClassTypes DLLStorage);
1011+
static bool isUsed(IRLinkage IRL);
9871012
};
9881013

9891014
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.
@@ -2886,8 +2872,8 @@ llvm::GlobalValue *IRGenModule::defineAlias(LinkEntity entity,
28862872
auto *alias = llvm::GlobalAlias::create(
28872873
ptrTy->getElementType(), ptrTy->getAddressSpace(), link.getLinkage(),
28882874
link.getName(), definition, &Module);
2889-
alias->setVisibility(link.getVisibility());
2890-
alias->setDLLStorageClass(link.getDLLStorage());
2875+
ApplyIRLinkage({link.getLinkage(), link.getVisibility(), link.getDLLStorage()})
2876+
.to(alias);
28912877

28922878
if (link.isUsed()) {
28932879
addUsedGlobal(alias);
@@ -3815,9 +3801,10 @@ static llvm::Function *shouldDefineHelper(IRGenModule &IGM,
38153801
if (!def) return nullptr;
38163802
if (!def->empty()) return nullptr;
38173803

3818-
def->setLinkage(llvm::Function::LinkOnceODRLinkage);
3819-
def->setVisibility(llvm::Function::HiddenVisibility);
3820-
def->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
3804+
ApplyIRLinkage({llvm::GlobalValue::LinkOnceODRLinkage,
3805+
llvm::GlobalValue::HiddenVisibility,
3806+
llvm::GlobalValue::DefaultStorageClass})
3807+
.to(def);
38213808
def->setDoesNotThrow();
38223809
def->setCallingConv(IGM.DefaultCC);
38233810
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)