https://github.com/mtrofin updated https://github.com/llvm/llvm-project/pull/203171
>From 1d27dab1cb758edaafb9801f6307d3a70ef1cabf Mon Sep 17 00:00:00 2001 From: Mircea Trofin <[email protected]> Date: Wed, 17 Jun 2026 11:06:09 -0700 Subject: [PATCH] [CFI] Create an external linkage alias instead of promoting internals --- llvm/lib/Transforms/IPO/LowerTypeTests.cpp | 35 +++++++++++++ .../Transforms/IPO/ThinLTOBitcodeWriter.cpp | 52 +++++++------------ .../lib/Transforms/IPO/WholeProgramDevirt.cpp | 25 +++++++-- .../ThinLTO/X86/devirt_function_alias2.ll | 10 ++-- .../LowerTypeTests/promoted-internal.ll | 29 +++++++++++ .../cfi-icall-static-inline-asm.ll | 4 +- .../Transforms/ThinLTOBitcodeWriter/comdat.ll | 17 +++--- .../ThinLTOBitcodeWriter/split-internal1.ll | 3 +- .../ThinLTOBitcodeWriter/split-internal2.ll | 5 +- .../split-vfunc-internal.ll | 6 ++- 10 files changed, 130 insertions(+), 56 deletions(-) create mode 100644 llvm/test/Transforms/LowerTypeTests/promoted-internal.ll diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp index 7c045c90ead5b..9469934b8d5e7 100644 --- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp +++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp @@ -2138,6 +2138,38 @@ bool LowerTypeTestsModule::lower() { report_fatal_error( "unexpected call to llvm.icall.branch.funnel during import phase"); + // For internal linkage cfiFunction defs/decls, we only needed the alias + // through the linker. We can replace those aliases with the aliased + // function here. + SmallVector<std::pair<Function *, std::string>> PromotedFuncs; + for (auto &A : llvm::make_early_inc_range(M.aliases())) { + if (A.hasLocalLinkage()) + continue; + if (ImportSummary->cfiFunctionDefs().contains(A.getName()) || + ImportSummary->cfiFunctionDecls().contains(A.getName())) { + if (auto *F = dyn_cast_or_null<Function>(A.getAliaseeObject())) { + if (F->hasExternalLinkage()) { + // The original internal linkage function was independently promoted + // by thinlink. While, pre-link, all static references to it + // (implicitly, module-internal) were replaced with references to + // the alias, thinlink might decide to promote it because (for + // example) it turns out to be a hot indirect call target in a + // different module. + // In that case, we need to remember its thinlink-promoted name + // because it's potentially referenced elsewhere, and make sure + // there's an alias to it. + PromotedFuncs.emplace_back(F, F->getName()); + } else { + F->setLinkage(GlobalValue::ExternalLinkage); + F->setVisibility(GlobalValue::HiddenVisibility); + } + A.replaceAllUsesWith(F); + F->takeName(&A); + A.eraseFromParent(); + } + } + } + SmallVector<Function *, 8> Defs; SmallVector<Function *, 8> Decls; for (auto &F : M) { @@ -2158,6 +2190,9 @@ bool LowerTypeTestsModule::lower() { for (auto *F : Decls) importFunction(F, /*isJumpTableCanonical*/ false); } + // Add an alias with the thinlink promotion name. + for (auto &[F, Name] : PromotedFuncs) + GlobalAlias::create(GlobalValue::LinkageTypes::ExternalLinkage, Name, F); return true; } diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp index 40200a6554249..d784cc328fb99 100644 --- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp @@ -30,23 +30,10 @@ using namespace llvm; namespace { -// Determine if a promotion alias should be created for a symbol name. -static bool allowPromotionAlias(const std::string &Name) { - // Promotion aliases are used only in inline assembly. It's safe to - // simply skip unusual names. Subset of MCAsmInfo::isAcceptableChar() - // and MCAsmInfoXCOFF::isAcceptableChar(). - for (const char &C : Name) { - if (isAlnum(C) || C == '_' || C == '.') - continue; - return false; - } - return true; -} - // Promote each local-linkage entity defined by ExportM and used by ImportM by // changing visibility and appending the given ModuleId. void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId, - const SetVector<GlobalValue *> &PromoteExtra) { + SetVector<GlobalValue *> *PromoteExtra = nullptr) { DenseMap<const Comdat *, Comdat *> RenamedComdats; for (auto &ExportGV : ExportM.global_values()) { if (!ExportGV.hasLocalLinkage()) @@ -54,7 +41,8 @@ void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId, auto Name = ExportGV.getName(); GlobalValue *ImportGV = nullptr; - if (!PromoteExtra.count(&ExportGV)) { + const bool MustPromote = PromoteExtra && PromoteExtra->count(&ExportGV); + if (!MustPromote) { ImportGV = ImportM.getNamedValue(Name); if (!ImportGV) continue; @@ -72,24 +60,23 @@ void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId, if (C->getName() == Name) RenamedComdats.try_emplace(C, ExportM.getOrInsertComdat(NewName)); - ExportGV.setName(NewName); - ExportGV.setLinkage(GlobalValue::ExternalLinkage); - ExportGV.setVisibility(GlobalValue::HiddenVisibility); - // TODO: remove this reassign and instead create an alias. - ExportGV.reassignGUID(); + auto *ExternalAlias = GlobalAlias::create( + ExportGV.getType(), ExportGV.getAddressSpace(), + GlobalValue::ExternalLinkage, NewName, &ExportGV, &ExportM); + ExternalAlias->setVisibility(GlobalValue::HiddenVisibility); + ExportGV.replaceUsesWithIf( + ExternalAlias, [](Use &U) { return !isa<GlobalAlias>(U.getUser()); }); + + if (MustPromote) { + PromoteExtra->remove(&ExportGV); + PromoteExtra->insert(ExternalAlias); + } + if (ImportGV) { ImportGV->setName(NewName); ImportGV->setVisibility(GlobalValue::HiddenVisibility); ImportGV->reassignGUID(); } - - if (isa<Function>(&ExportGV) && allowPromotionAlias(OldName)) { - // Create a local alias with the original name to avoid breaking - // references from inline assembly. - std::string Alias = - ".lto_set_conditional " + OldName + "," + NewName + "\n"; - ExportM.appendModuleInlineAsm(Alias); - } } if (!RenamedComdats.empty()) @@ -191,9 +178,8 @@ void simplifyExternals(Module &M) { F.getName().starts_with("llvm.")) continue; - Function *NewF = - Function::Create(EmptyFT, GlobalValue::ExternalLinkage, - F.getAddressSpace(), "", &M); + Function *NewF = Function::Create(EmptyFT, GlobalValue::ExternalLinkage, + F.getAddressSpace(), "", &M); NewF->copyAttributesFrom(&F); // Only copy function attribtues. NewF->setAttributes(AttributeList::get(M.getContext(), @@ -419,8 +405,8 @@ void splitAndWriteThinLTOBitcode( // match values from its first argument (the "exporting module") in // CfiFunctions. So we only need CfiFunctions for the second promotion (M -> // MergedM) - promoteInternals(*MergedM, M, ModuleId, {}); - promoteInternals(M, *MergedM, ModuleId, CfiFunctions); + promoteInternals(*MergedM, M, ModuleId, nullptr); + promoteInternals(M, *MergedM, ModuleId, &CfiFunctions); auto &Ctx = MergedM->getContext(); SmallVector<MDNode *, 8> CfiFunctionMDs; diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index 0c8b2aa7078c2..62cacb7bb791c 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -1147,6 +1147,8 @@ bool DevirtModule::tryFindVirtualCallTargets( // target. auto *GV = dyn_cast<GlobalValue>(C); assert(GV); + if (auto *GA = dyn_cast<GlobalAlias>(GV)) + GV = GA->getAliaseeObject(); TargetsForSlot.push_back({GV, &TM}); } @@ -1203,7 +1205,17 @@ bool DevirtIndex::tryFindVirtualCallTargets( if (mustBeUnreachableFunction(VTP.FuncVI)) continue; - TargetsForSlot.push_back(VTP.FuncVI); + ValueInfo TheFn = VTP.FuncVI; + // Internal linkage functions get an external linkage alias to them. + if (!TheFn.getSummaryList().empty()) + if (auto *AS = + dyn_cast<AliasSummary>(TheFn.getSummaryList()[0].get())) { + assert(TheFn.getSummaryList().size() == 1 && + "Aliases to internal linkage targets of indirect calls are " + "expected to point to exactly one definition."); + TheFn = AS->getAliaseeVI(); + } + TargetsForSlot.push_back(TheFn); } } @@ -2261,10 +2273,13 @@ void DevirtModule::importResolution(VTableSlot Slot, VTableSlotInfo &SlotInfo) { assert(!Res.SingleImplName.empty()); // The type of the function in the declaration is irrelevant because every // call site will cast it to the correct type. - Constant *SingleImpl = - cast<Constant>(M.getOrInsertFunction(Res.SingleImplName, - Type::getVoidTy(M.getContext())) - .getCallee()); + Value *SingleImplVal = + M.getOrInsertFunction(Res.SingleImplName, + Type::getVoidTy(M.getContext())) + .getCallee(); + if (auto *A = dyn_cast<GlobalAlias>(SingleImplVal->stripPointerCasts())) + SingleImplVal = A->getAliaseeObject(); + Constant *SingleImpl = cast<Constant>(SingleImplVal); // This is the import phase so we should not be exporting anything. bool IsExported = false; diff --git a/llvm/test/ThinLTO/X86/devirt_function_alias2.ll b/llvm/test/ThinLTO/X86/devirt_function_alias2.ll index 3b3208e300aa8..e4f93208fba54 100644 --- a/llvm/test/ThinLTO/X86/devirt_function_alias2.ll +++ b/llvm/test/ThinLTO/X86/devirt_function_alias2.ll @@ -24,8 +24,8 @@ ; RUN: 2>&1 | FileCheck %s --check-prefix=REMARK --check-prefix=PRINT ; RUN: llvm-dis %t2.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR1 -; PRINT-DAG: Devirtualized call to {{.*}} (_ZN1D1mEiAlias) -; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEiAlias +; PRINT-DAG: Devirtualized call to {{.*}} (_ZN1D1mEi) +; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi ;; Test hybrid Thin/Regular LTO @@ -63,13 +63,15 @@ target triple = "x86_64-grtev4-linux-gnu" @_ZTV1D = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1D1mEiAlias] }, !type !3 -define i32 @_ZN1D1mEi(ptr %this, i32 %a) { +define i32 @_ZN1D1mEi(ptr %this, i32 %a) #0 { ret i32 0; } @_ZN1D1mEiAlias = unnamed_addr alias i32 (ptr, i32), ptr @_ZN1D1mEi -; CHECK-IR1-LABEL: define i32 @test +attributes #0 = { noinline optnone } + +; CHECK-IR1-LABEL: define {{.*}}i32 @test define i32 @test(ptr %obj2, i32 %a) { entry: %vtable2 = load ptr, ptr %obj2 diff --git a/llvm/test/Transforms/LowerTypeTests/promoted-internal.ll b/llvm/test/Transforms/LowerTypeTests/promoted-internal.ll new file mode 100644 index 0000000000000..f4cdadd90f6ab --- /dev/null +++ b/llvm/test/Transforms/LowerTypeTests/promoted-internal.ll @@ -0,0 +1,29 @@ +; RUN: split-file %s %t +; RUN: opt -passes=lowertypetests -lowertypetests-summary-action=import \ +; RUN: -lowertypetests-read-summary=%t/import.yaml %t/module.ll -S -o - | FileCheck %s + +; CHECK: @f.llvm.1234 = hidden alias ptr, ptr @f.5678.cfi +; CHECK: define hidden void @f.5678.cfi() +; CHECK: declare void @f.5678() + +;--- import.yaml +CfiFunctionDefs: +- Name: f.5678 + GUID: 4670599147315008938 +--- + +;--- module.ll +source_filename = "promoted-internal.ll" + +; f.5678 GUID: 4670599147315008938 [email protected] = hidden alias ptr, ptr @f.llvm.1234 + +define void @f.llvm.1234() !type !0 !guid !{i64 1234} { + ret void +} + +!cfi.functions = !{!1} +!aliases = !{!2} +!0 = !{i64 0, !"_ZTSFvE"} +!1 = !{!"f.5678", i8 0, i64 4670599147315008938, !1} +!2 = !{!"f", !"f.5678"} diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll index d8ebae17d4693..9d2ee425031ab 100644 --- a/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll @@ -3,7 +3,7 @@ target triple = "x86_64-unknown-linux-gnu" -; CHECK: module asm ".lto_set_conditional a,a.[[HASH:[0-9a-f]+]]" +; CHECK: @a.[[HASH:[0-9a-f]+]] = hidden alias define void @b() { %f = alloca ptr, align 8 @@ -14,7 +14,7 @@ define void @b() { ret void } -; CHECK: define{{.*}} @a.[[HASH]](){{.*}} !type +; CHECK: define internal void @a() {{.*}}!type define internal void @a() !type !0 { ret void } diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll index f9545e2010c10..48d371531ed7b 100644 --- a/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll @@ -25,9 +25,8 @@ $nt = comdat any ; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) @lwt_aliasee = private unnamed_addr global [1 x ptr] [ptr null], comdat($lwt), !type !0 -; MERGED: {{@"?lwt_nl[^ ]+}} = hidden unnamed_addr global -; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) -; THIN: {{@"?lwt_nl[^ ]+}} = external hidden +; MERGED: @lwt_nl = internal unnamed_addr global i32 0, comdat(${{"?lwt[^ ]+}}) +; THIN: {{@"?lwt_nl\.[^ ]+}} = external hidden unnamed_addr global i32 @lwt_nl = internal unnamed_addr global i32 0, comdat($lwt) ; MERGED: @nlwt_aliasee = private unnamed_addr global @@ -47,12 +46,16 @@ $nt = comdat any ; THIN-SAME: comdat($nt) @nt_nl = internal unnamed_addr global i32 0, comdat($nt) -; MERGED: {{@"?lwt[^ ]+}} = hidden unnamed_addr alias -; THIN: {{@"?lwt[^ ]+}} = external hidden +; MERGED: @lwt = internal unnamed_addr alias [1 x ptr], ptr @lwt_aliasee +; MERGED: @nlwt_nl = internal unnamed_addr alias [1 x ptr], ptr @nlwt_aliasee +; MERGED: {{@"?lwt_nl\.[^ ]+}} = hidden alias ptr, ptr @lwt_nl +; MERGED: {{@"?lwt\.[^ ]+}} = hidden alias ptr, ptr @lwt +; MERGED: {{@"?nlwt_nl\.[^ ]+}} = hidden alias ptr, ptr @nlwt_nl + +; THIN: {{@"?lwt\.[^ ]+}} = external hidden global [1 x ptr] @lwt = internal unnamed_addr alias [1 x ptr], ptr @lwt_aliasee -; MERGED: {{@"?nlwt_nl[^ ]+}} = hidden unnamed_addr alias -; THIN: {{@"?nlwt_nl[^ ]+}} = external hidden +; THIN: {{@"?nlwt_nl\.[^ ]+}} = external hidden global [1 x ptr] @nlwt_nl = internal unnamed_addr alias [1 x ptr], ptr @nlwt_aliasee ; The functions below exist just to make sure the globals are used. diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal1.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal1.ll index 1a6cbb7e7e84b..2c84ab86cac63 100644 --- a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal1.ll +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal1.ll @@ -13,7 +13,8 @@ ; BCA1-NOT: <GLOBALVAL_SUMMARY_BLOCK ; M0: @g.581d7631532fa146ba4061179da39272 = external hidden global i8 -; M1: @g.581d7631532fa146ba4061179da39272 = hidden global i8 42, !type !0 +; M1: @g = internal global i8 42, !type !0 +; M1: @g.581d7631532fa146ba4061179da39272 = hidden alias ptr, ptr @g @g = internal global i8 42, !type !0 ; M0: define ptr @f() diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal2.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal2.ll index 1a7b1aba66576..d9d9f516015d9 100644 --- a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal2.ll +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-internal2.ll @@ -20,8 +20,9 @@ target triple = "x86_64-unknown-linux-gnu" ; M1: @g = global ptr @f.13757e0fb71915e385efa4dc9d1e08fd, !type !0 @g = global ptr @f, !type !0 -; M0: define hidden void @f.13757e0fb71915e385efa4dc9d1e08fd() -; M1: declare !guid !4 hidden void @f.13757e0fb71915e385efa4dc9d1e08fd() +; M0: @f.13757e0fb71915e385efa4dc9d1e08fd = hidden alias ptr, ptr @f +; M0: define internal void @f() +; M1: declare !guid !{{[0-9]+}} hidden void @f.13757e0fb71915e385efa4dc9d1e08fd() define internal void @f() { call void @f2() ret void diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-vfunc-internal.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-vfunc-internal.ll index 75932fd84d83e..1d17699be1c3c 100644 --- a/llvm/test/Transforms/ThinLTOBitcodeWriter/split-vfunc-internal.ll +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/split-vfunc-internal.ll @@ -10,12 +10,14 @@ define ptr @source() { } ; M0: @g.84f59439b469192440047efc8de357fb = external hidden constant [1 x ptr] -; M1: @g.84f59439b469192440047efc8de357fb = hidden constant [1 x ptr] [ptr @ok.84f59439b469192440047efc8de357fb] +; M1: @g = internal constant [1 x ptr] [ptr @ok.84f59439b469192440047efc8de357fb] +; M1: @g.84f59439b469192440047efc8de357fb = hidden alias ptr, ptr @g @g = internal constant [1 x ptr] [ ptr @ok ], !type !0 -; M0: define hidden i64 @ok.84f59439b469192440047efc8de357fb +; M0: @ok.84f59439b469192440047efc8de357fb = hidden alias ptr, ptr @ok +; M0: define internal i64 @ok(ptr %this) ; M1: define available_externally hidden i64 @ok.84f59439b469192440047efc8de357fb define internal i64 @ok(ptr %this) { ret i64 42 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
