[WebAssembly] Don't remove function signatures in ThinLTOBitcodeWriter #126552
+51
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the split LTO unit mode in ThinLTO, functions that meet the a specific criteria are moved to the split module, but also the declarations for the other functions that are still in the oritinal module are written to the split module. In `simplifyExternals` in `ThinLTOBitcodeWriter.cpp`, the function signatures are removed from those declarations in the split module, presumably in order to reduce code size or they are not necessary for other targets. But Wasm needs them, because if function signatures don't match from two modules, function signature mismatch warnings like this occur in the linker: ``` wasm-ld: warning: function signature mismatch: _ZN12_GLOBAL__N_116itanium_demangle19PointerToMemberTypeD0Ev.cb45941bcc851b6faf99a8b8c1ca5f22 >>> defined as () -> void in /usr/local/google/home/aheejin/emscripten/cache/sysroot/lib/wasm32-emscripten/thinlto/libc++abi-wasmexcept.a(cxa_demangle.o) >>> defined as (i32) -> void in lto.tmp ``` And this can result in an error at runtime. This skips the part that removes function signatures from `simplifyExternals` when the architecture is Wasm.
llvmbot
added
LTO
|
@llvm/pr-subscribers-llvm-transforms Author: Heejin Ahn (aheejin) ChangesIn the split LTO unit mode in ThinLTO, functions that meet the a specific criteria are moved to the split module, but also the declarations for the other functions that are still in the oritinal module are written to the split module. In And this can result in an error at runtime. This skips the part that removes function signatures from Full diff: https://github.com/llvm/llvm-project/pull/126552.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index cd0e412bdf353b2..2dc60e8f168e650 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -178,28 +178,29 @@ void simplifyExternals(Module &M) {
FunctionType *EmptyFT =
FunctionType::get(Type::getVoidTy(M.getContext()), false);
- for (Function &F : llvm::make_early_inc_range(M)) {
- if (F.isDeclaration() && F.use_empty()) {
+ for (Function &F : llvm::make_early_inc_range(M))
+ if (F.isDeclaration() && F.use_empty())
F.eraseFromParent();
- continue;
- }
- if (!F.isDeclaration() || F.getFunctionType() == EmptyFT ||
- // Changing the type of an intrinsic may invalidate the IR.
- F.getName().starts_with("llvm."))
- continue;
+ // Wasm should preserve function signatures for the linker.
+ if (!Triple(M.getTargetTriple()).isWasm()) {
+ for (Function &F : llvm::make_early_inc_range(M)) {
+ if (!F.isDeclaration() || F.getFunctionType() == EmptyFT ||
+ // Changing the type of an intrinsic may invalidate the IR.
+ F.getName().starts_with("llvm."))
+ continue;
- Function *NewF =
- Function::Create(EmptyFT, GlobalValue::ExternalLinkage,
- F.getAddressSpace(), "", &M);
- NewF->copyAttributesFrom(&F);
- // Only copy function attribtues.
- NewF->setAttributes(AttributeList::get(M.getContext(),
- AttributeList::FunctionIndex,
- F.getAttributes().getFnAttrs()));
- NewF->takeName(&F);
- F.replaceAllUsesWith(NewF);
- F.eraseFromParent();
+ Function *NewF = Function::Create(EmptyFT, GlobalValue::ExternalLinkage,
+ F.getAddressSpace(), "", &M);
+ NewF->copyAttributesFrom(&F);
+ // Only copy function attribtues.
+ NewF->setAttributes(AttributeList::get(M.getContext(),
+ AttributeList::FunctionIndex,
+ F.getAttributes().getFnAttrs()));
+ NewF->takeName(&F);
+ F.replaceAllUsesWith(NewF);
+ F.eraseFromParent();
+ }
}
for (GlobalIFunc &I : llvm::make_early_inc_range(M.ifuncs())) {
diff --git a/llvm/test/ThinLTO/WebAssembly/lit.local.cfg b/llvm/test/ThinLTO/WebAssembly/lit.local.cfg
new file mode 100644
index 000000000000000..d5f39ab4dbc8ca2
--- /dev/null
+++ b/llvm/test/ThinLTO/WebAssembly/lit.local.cfg
@@ -0,0 +1,2 @@
+if not "WebAssembly" in config.root.targets:
+ config.unsupported = True
diff --git a/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll b/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll
new file mode 100644
index 000000000000000..201aa81c3007371
--- /dev/null
+++ b/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll
@@ -0,0 +1,29 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %s -o %t.o
+; RUN: llvm-modextract -b -n 0 %t.o -o - | llvm-dis | FileCheck %s --check-prefix=UNIT0
+; RUN: llvm-modextract -b -n 1 %t.o -o - | llvm-dis | FileCheck %s --check-prefix=UNIT1
+
+target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+target triple = "wasm32-unknown-unknown"
+
+; After split LTO unit generation, @test_gv will move to UNIT1 (because it has a
+; type metadata), and because it refers to @test, @test will be promoted by
+; adding its module ID hash to its name, and .lto_set_conditional directive will
+; be written in Unit 0.
+
+; UNIT0: module asm ".lto_set_conditional test,test.{{[0-9a-f]+}}"
+; UNIT0: define hidden i32 @test.{{[0-9a-f]+}}()
+
+; Unit 1 will contain @test's declaration. The normal ThinLTO split bitcode
+; writing removes the signatures from the split unit's declaration, but in Wasm
+; you should not do this in order to avoid function signature mismatch in
+; linker.
+
+; UNIT1: declare hidden i32 @test.{{[0-9a-f]+}}()
+
+@test_gv = constant ptr @test, align 4, !type !0
+
+define internal i32 @test() {
+ ret i32 0
+}
+
+!0 = !{i64 0, !{}}
|
|
@llvm/pr-subscribers-lto Author: Heejin Ahn (aheejin) ChangesIn the split LTO unit mode in ThinLTO, functions that meet the a specific criteria are moved to the split module, but also the declarations for the other functions that are still in the oritinal module are written to the split module. In And this can result in an error at runtime. This skips the part that removes function signatures from Full diff: https://github.com/llvm/llvm-project/pull/126552.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
index cd0e412bdf353b2..2dc60e8f168e650 100644
--- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -178,28 +178,29 @@ void simplifyExternals(Module &M) {
FunctionType *EmptyFT =
FunctionType::get(Type::getVoidTy(M.getContext()), false);
- for (Function &F : llvm::make_early_inc_range(M)) {
- if (F.isDeclaration() && F.use_empty()) {
+ for (Function &F : llvm::make_early_inc_range(M))
+ if (F.isDeclaration() && F.use_empty())
F.eraseFromParent();
- continue;
- }
- if (!F.isDeclaration() || F.getFunctionType() == EmptyFT ||
- // Changing the type of an intrinsic may invalidate the IR.
- F.getName().starts_with("llvm."))
- continue;
+ // Wasm should preserve function signatures for the linker.
+ if (!Triple(M.getTargetTriple()).isWasm()) {
+ for (Function &F : llvm::make_early_inc_range(M)) {
+ if (!F.isDeclaration() || F.getFunctionType() == EmptyFT ||
+ // Changing the type of an intrinsic may invalidate the IR.
+ F.getName().starts_with("llvm."))
+ continue;
- Function *NewF =
- Function::Create(EmptyFT, GlobalValue::ExternalLinkage,
- F.getAddressSpace(), "", &M);
- NewF->copyAttributesFrom(&F);
- // Only copy function attribtues.
- NewF->setAttributes(AttributeList::get(M.getContext(),
- AttributeList::FunctionIndex,
- F.getAttributes().getFnAttrs()));
- NewF->takeName(&F);
- F.replaceAllUsesWith(NewF);
- F.eraseFromParent();
+ Function *NewF = Function::Create(EmptyFT, GlobalValue::ExternalLinkage,
+ F.getAddressSpace(), "", &M);
+ NewF->copyAttributesFrom(&F);
+ // Only copy function attribtues.
+ NewF->setAttributes(AttributeList::get(M.getContext(),
+ AttributeList::FunctionIndex,
+ F.getAttributes().getFnAttrs()));
+ NewF->takeName(&F);
+ F.replaceAllUsesWith(NewF);
+ F.eraseFromParent();
+ }
}
for (GlobalIFunc &I : llvm::make_early_inc_range(M.ifuncs())) {
diff --git a/llvm/test/ThinLTO/WebAssembly/lit.local.cfg b/llvm/test/ThinLTO/WebAssembly/lit.local.cfg
new file mode 100644
index 000000000000000..d5f39ab4dbc8ca2
--- /dev/null
+++ b/llvm/test/ThinLTO/WebAssembly/lit.local.cfg
@@ -0,0 +1,2 @@
+if not "WebAssembly" in config.root.targets:
+ config.unsupported = True
diff --git a/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll b/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll
new file mode 100644
index 000000000000000..201aa81c3007371
--- /dev/null
+++ b/llvm/test/ThinLTO/WebAssembly/thinlto-split-lto-unit.ll
@@ -0,0 +1,29 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit %s -o %t.o
+; RUN: llvm-modextract -b -n 0 %t.o -o - | llvm-dis | FileCheck %s --check-prefix=UNIT0
+; RUN: llvm-modextract -b -n 1 %t.o -o - | llvm-dis | FileCheck %s --check-prefix=UNIT1
+
+target datalayout = "e-m:e-p:32:32-p10:8:8-p20:8:8-i64:64-n32:64-S128-ni:1:10:20"
+target triple = "wasm32-unknown-unknown"
+
+; After split LTO unit generation, @test_gv will move to UNIT1 (because it has a
+; type metadata), and because it refers to @test, @test will be promoted by
+; adding its module ID hash to its name, and .lto_set_conditional directive will
+; be written in Unit 0.
+
+; UNIT0: module asm ".lto_set_conditional test,test.{{[0-9a-f]+}}"
+; UNIT0: define hidden i32 @test.{{[0-9a-f]+}}()
+
+; Unit 1 will contain @test's declaration. The normal ThinLTO split bitcode
+; writing removes the signatures from the split unit's declaration, but in Wasm
+; you should not do this in order to avoid function signature mismatch in
+; linker.
+
+; UNIT1: declare hidden i32 @test.{{[0-9a-f]+}}()
+
+@test_gv = constant ptr @test, align 4, !type !0
+
+define internal i32 @test() {
+ ret i32 0
+}
+
+!0 = !{i64 0, !{}}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the split LTO unit mode in ThinLTO, functions that meet the a specific criteria are moved to the split module, but also the declarations for the other functions that are still in the oritinal module are written to the split module.
In
simplifyExternalsinThinLTOBitcodeWriter.cpp, the function signatures are removed from those declarations in the split module, presumably in order to reduce code size or they are not necessary for other targets. But Wasm needs them, because if function signatures don't match from two modules, function signature mismatch warnings like this occur in the linker:And this can result in an error at runtime.
This skips the part that removes function signatures from
simplifyExternalswhen the architecture is Wasm.