[llvm] [ThinLTO]Supports declaration import for global variables in distributed ThinLTO (PR #117616)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 25 11:35:04 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir
Author: Mingming Liu (mingmingl-llvm)
<details>
<summary>Changes</summary>
When `-import-declaration` option is enabled, declaration import is supported for functions. https://github.com/llvm/llvm-project/pull/88024 has the context for this option.
This patch supports declaration import for global variables in distributed ThinLTO. The motivating use case is to propagate `dso_local` attribute of global variables across modules, to optimize global variable access when a binary is built with `-fno-direct-access-external-data`.
* With `-fdirect-access-external-data`, non thread-local global variables will [have `dso_local` attributes](https://github.com/llvm/llvm-project/blob/fe3c23b439b9a2d00442d9bc6a4ca86f73066a3d/clang/lib/CodeGen/CodeGenModule.cpp#L1730-L1746). This optimizes the global variable access as shown by https://gcc.godbolt.org/z/vMzWcKdh3
---
Full diff: https://github.com/llvm/llvm-project/pull/117616.diff
5 Files Affected:
- (modified) llvm/include/llvm/IR/ModuleSummaryIndex.h (+5)
- (modified) llvm/lib/Bitcode/Writer/BitcodeWriter.cpp (+2-1)
- (modified) llvm/lib/IR/ModuleSummaryIndex.cpp (+16-2)
- (modified) llvm/lib/Transforms/IPO/FunctionImport.cpp (+10-2)
- (modified) llvm/test/ThinLTO/X86/import_callee_declaration.ll (+42-6)
``````````diff
diff --git a/llvm/include/llvm/IR/ModuleSummaryIndex.h b/llvm/include/llvm/IR/ModuleSummaryIndex.h
index 39c60229aa1d81..259391cbb718ba 100644
--- a/llvm/include/llvm/IR/ModuleSummaryIndex.h
+++ b/llvm/include/llvm/IR/ModuleSummaryIndex.h
@@ -1909,6 +1909,11 @@ class ModuleSummaryIndex {
/// Checks if we can import global variable from another module.
bool canImportGlobalVar(const GlobalValueSummary *S, bool AnalyzeRefs) const;
+
+ /// Same as above but checks whether the global var is importable as a
+ /// declaration.
+ bool canImportGlobalVar(const GlobalValueSummary *S, bool AnalyzeRefs,
+ bool &CanImportDecl) const;
};
/// GraphTraits definition to build SCC for the index
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index 24a4c2e8303d5a..dd6b97d521c4ae 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4764,7 +4764,8 @@ void IndexBitcodeWriter::writeCombinedGlobalValueSummary() {
NameVals.push_back(*ValueId);
assert(ModuleIdMap.count(VS->modulePath()));
NameVals.push_back(ModuleIdMap[VS->modulePath()]);
- NameVals.push_back(getEncodedGVSummaryFlags(VS->flags()));
+ NameVals.push_back(
+ getEncodedGVSummaryFlags(VS->flags(), shouldImportValueAsDecl(VS)));
NameVals.push_back(getEncodedGVarFlags(VS->varflags()));
for (auto &RI : VS->refs()) {
auto RefValueId = getValueId(RI.getGUID());
diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index 12a558b3bc1b12..d9024b0a8673f1 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/lib/IR/ModuleSummaryIndex.cpp
@@ -328,6 +328,13 @@ void ModuleSummaryIndex::propagateAttributes(
bool ModuleSummaryIndex::canImportGlobalVar(const GlobalValueSummary *S,
bool AnalyzeRefs) const {
+ bool CanImportDecl;
+ return canImportGlobalVar(S, AnalyzeRefs, CanImportDecl);
+}
+
+bool ModuleSummaryIndex::canImportGlobalVar(const GlobalValueSummary *S,
+ bool AnalyzeRefs,
+ bool &CanImportDecl) const {
auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {
// We don't analyze GV references during attribute propagation, so
// GV with non-trivial initializer can be marked either read or
@@ -348,13 +355,20 @@ bool ModuleSummaryIndex::canImportGlobalVar(const GlobalValueSummary *S,
};
auto *GVS = cast<GlobalVarSummary>(S->getBaseObject());
+ const bool nonInterposable =
+ !GlobalValue::isInterposableLinkage(S->linkage());
+ const bool eligibleToImport = !S->notEligibleToImport();
+
+ // It's correct to import a global variable only when it is not interposable
+ // and eligible to import.
+ CanImportDecl = (nonInterposable && eligibleToImport);
+
// Global variable with non-trivial initializer can be imported
// if it's readonly. This gives us extra opportunities for constant
// folding and converting indirect calls to direct calls. We don't
// analyze GV references during attribute propagation, because we
// don't know yet if it is readonly or not.
- return !GlobalValue::isInterposableLinkage(S->linkage()) &&
- !S->notEligibleToImport() &&
+ return nonInterposable && eligibleToImport &&
(!AnalyzeRefs || !HasRefsPreventingImport(GVS));
}
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index fee27f72f208b0..ca15a4b9e1be84 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -430,10 +430,18 @@ class GlobalsImporter final {
// than as part of the logic deciding which functions to import (i.e.
// based on profile information). Should we decide to handle them here,
// we can refactor accordingly at that time.
- if (!GVS || !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true) ||
+ bool CanImportDecl = false;
+ if (!GVS ||
shouldSkipLocalInAnotherModule(GVS, VI.getSummaryList().size(),
- Summary.modulePath()))
+ Summary.modulePath()) ||
+ !Index.canImportGlobalVar(GVS, /* AnalyzeRefs */ true,
+ CanImportDecl)) {
+ if (ImportDeclaration && CanImportDecl)
+ ImportList.maybeAddDeclaration(RefSummary->modulePath(),
+ VI.getGUID());
+
continue;
+ }
// If there isn't an entry for GUID, insert <GUID, Definition> pair.
// Otherwise, definition should take precedence over declaration.
diff --git a/llvm/test/ThinLTO/X86/import_callee_declaration.ll b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
index 246920e5db0dc8..fd62aeb3f74401 100644
--- a/llvm/test/ThinLTO/X86/import_callee_declaration.ll
+++ b/llvm/test/ThinLTO/X86/import_callee_declaration.ll
@@ -34,11 +34,15 @@
; RUN: -r=main.bc,main,px \
; RUN: -r=main.bc,small_func, \
; RUN: -r=main.bc,large_func, \
+; RUN: -r=main.bc,read_write_global_vars, \
+; RUN: -r=main.bc,external_func, \
; RUN: -r=lib.bc,callee,pl \
; RUN: -r=lib.bc,large_indirect_callee,px \
; RUN: -r=lib.bc,large_indirect_bar,px \
; RUN: -r=lib.bc,small_func,px \
; RUN: -r=lib.bc,large_func,px \
+; RUN: -r=lib.bc,read_write_global_vars,px \
+; RUN: -r=lib.bc,external_func, \
; RUN: -r=lib.bc,large_indirect_callee_alias,px \
; RUN: -r=lib.bc,large_indirect_bar_alias,px \
; RUN: -r=lib.bc,calleeAddrs,px -r=lib.bc,calleeAddrs2,px -o summary main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=DUMP
@@ -46,7 +50,7 @@
; RUN: llvm-lto -thinlto-action=thinlink -import-declaration -import-instr-limit=7 -import-instr-evolution-factor=1.0 -o combined.index.bc main.bc lib.bc
; RUN: llvm-lto -thinlto-action=distributedindexes -debug-only=function-import -import-declaration -import-instr-limit=7 -import-instr-evolution-factor=1.0 -thinlto-index combined.index.bc main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=DUMP
-; DUMP: - 2 function definitions and 4 function declarations imported from lib.bc
+; DUMP: - 2 function definitions and 3 function declarations imported from lib.bc
; First disassemble per-module summary and find out the GUID for {large_func, large_indirect_callee}.
;
@@ -67,17 +71,26 @@
; MAIN-DIS: gv: (guid: 2418497564662708935, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
; When alias is imported as a copy of the aliasee, but the aliasee is not being
; imported by itself, the aliasee should be null.
-; MAIN-DIS: gv: (guid: 13590951773474913315, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: null)))
+; MAIN-DIS-NOT: gv: (guid: 13590951773474913315, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: null)))
; MAIN-DIS: [[LARGEINDIRECT:\^[0-9]+]] = gv: (guid: 14343440786664691134, summaries: (function: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), insts: 8, {{.*}})))
; MAIN-DIS: gv: (guid: 16730173943625350469, summaries: (alias: (module: [[LIBMOD]], flags: ({{.*}} importType: declaration), aliasee: [[LARGEINDIRECT]])))
+; RUN: opt -passes=function-import -summary-file=main.bc.thinlto.bc main.bc -o main-after-import.bc
+; RUN: llvm-dis -o - main-after-import.bc | FileCheck %s --check-prefix=MAIN-IMPORT
+
+; Tests that dso_local attribute is applied on a global var from its summary.
+MAIN-IMPORT: @read_write_global_vars = external dso_local global [1 x ptr]
+
; Run in-process ThinLTO and tests that
; 1. `callee` remains internalized even if the symbols of its callers
; (large_func, large_indirect_callee, large_indirect_bar) are exported as
; declarations and visible to main module.
; 2. the debugging logs from `function-import` pass are expected.
+; Set relocation model to static so the dso_local attribute from a summary is
+; applied on the global variable declaration.
; RUN: llvm-lto2 run \
+; RUN: -relocation-model=static \
; RUN: -debug-only=function-import \
; RUN: -save-temps \
; RUN: -thinlto-threads=1 \
@@ -87,11 +100,15 @@
; RUN: -r=main.bc,main,px \
; RUN: -r=main.bc,small_func, \
; RUN: -r=main.bc,large_func, \
+; RUN: -r=main.bc,read_write_global_vars, \
+; RUN: -r=main.bc,external_func, \
; RUN: -r=lib.bc,callee,pl \
; RUN: -r=lib.bc,large_indirect_callee,px \
; RUN: -r=lib.bc,large_indirect_bar,px \
; RUN: -r=lib.bc,small_func,px \
; RUN: -r=lib.bc,large_func,px \
+; RUN: -r=lib.bc,read_write_global_vars,px \
+; RUN: -r=lib.bc,external_func, \
; RUN: -r=lib.bc,large_indirect_callee_alias,px \
; RUN: -r=lib.bc,large_indirect_bar_alias,px \
; RUN: -r=lib.bc,calleeAddrs,px -r=lib.bc,calleeAddrs2,px -o in-process main.bc lib.bc 2>&1 | FileCheck %s --check-prefix=IMPORTDUMP
@@ -103,9 +120,9 @@
; IMPORTDUMP-DAG: Is importing function definition 13568239288960714650 small_indirect_callee from lib.cc
; IMPORTDUMP-DAG: Is importing function definition 6976996067367342685 small_func from lib.cc
; IMPORTDUMP-DAG: Is importing function declaration 2418497564662708935 large_func from lib.cc
-; IMPORTDUMP-DAG: Not importing global 7680325410415171624 calleeAddrs from lib.cc
+; IMPORTDUMP-DAG: Is importing global declaration 7680325410415171624 calleeAddrs from lib.cc
; IMPORTDUMP-DAG: Is importing alias declaration 16730173943625350469 large_indirect_callee_alias from lib.cc
-; IMPORTDUMP-DAG: Is importing alias declaration 13590951773474913315 large_indirect_bar_alias from lib.cc
+; IMPORTDUMP-DAG: Not importing alias 13590951773474913315 large_indirect_bar_alias from lib.cc
; IMPORTDUMP-DAG: Not importing function 13770917885399536773 large_indirect_bar
; RUN: llvm-dis in-process.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT
@@ -115,6 +132,8 @@
; IMPORT-DAG: define available_externally void @small_func
; IMPORT-DAG: define available_externally hidden void @small_indirect_callee
; IMPORT-DAG: declare void @large_func
+; Tests that dso_local attribute is applied on a global var from its summary.
+; IMPORT-DAG: @read_write_global_vars = external dso_local global [1 x ptr]
; IMPORT-NOT: large_indirect_callee
; IMPORT-NOT: large_indirect_callee_alias
; IMPORT-NOT: large_indirect_bar
@@ -126,9 +145,14 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
+ at read_write_global_vars = external global [1 x ptr]
+
define i32 @main() {
call void @small_func()
call void @large_func()
+ %num = call ptr @external_func(ptr @read_write_global_vars)
+ store ptr %num, ptr getelementptr inbounds ([1 x ptr], ptr @read_write_global_vars, i64 0, i64 0)
+ %res1 = call i32 @external_func(ptr @read_write_global_vars)
ret i32 0
}
@@ -137,6 +161,8 @@ declare void @small_func()
; large_func without attributes
declare void @large_func()
+declare ptr @external_func(ptr)
+
;--- lib.ll
source_filename = "lib.cc"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
@@ -149,6 +175,10 @@ target triple = "x86_64-unknown-linux-gnu"
; large_indirect_bar_alias is visible to main.ll but its aliasee isn't.
@calleeAddrs2 = global [1 x ptr] [ptr @large_indirect_bar_alias]
+; @read_write_global_vars is not read-only nor write-only (in main.ll). It's not
+; a constant global var and has references, so it's not importable as a definition.
+ at read_write_global_vars = dso_local global [1 x ptr] [ptr @large_indirect_callee]
+
define void @callee() #1 {
ret void
}
@@ -177,8 +207,12 @@ define void @large_indirect_bar()#2 {
define internal void @small_indirect_callee() #0 {
entry:
- %0 = load ptr, ptr @calleeAddrs2
- call void %0(), !prof !3
+ %0 = load ptr, ptr @calleeAddrs
+ ; The function-import pass crash (see pr/117584) when alias is imported but
+ ; aliasee isn't.
+ ; TODO: Update !prof to !3 (for @large_indirect_bar_alias) after alias/aliasee
+ ; import issue is handled.
+ call void %0(), !prof !0
ret void
}
@@ -197,6 +231,8 @@ entry:
ret void
}
+declare void @external_func()
+
define void @large_func() #0 {
entry:
call void @callee()
``````````
</details>
https://github.com/llvm/llvm-project/pull/117616
More information about the llvm-commits
mailing list