Skip to content

[WebAssemblyLowerEmscriptenEHSjLj] Avoid setting import_name where possible #128564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 25 additions & 20 deletions llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,22 +440,25 @@ static std::string getSignature(FunctionType *FTy) {
return Sig;
}

static Function *getEmscriptenFunction(FunctionType *Ty, const Twine &Name,
Module *M) {
Function* F = Function::Create(Ty, GlobalValue::ExternalLinkage, Name, M);
static Function *getFunction(FunctionType *Ty, const Twine &Name, Module *M) {
return Function::Create(Ty, GlobalValue::ExternalLinkage, Name, M);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this function really an improvement now over just calling Function::Create directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy to switch to that if you prefer?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it avoids typing GlobalValue::ExternalLinkage a lot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess I could go either way on that. LGTM for whatever you want to do.

}

static void markAsImported(Function *F) {
// Tell the linker that this function is expected to be imported from the
// 'env' module.
// 'env' module. This is necessary for functions that do not have fixed names
// (e.g. __import_xyz). These names cannot be provided by any kind of shared
// or static library as instead we mark them explictly as imported.
if (!F->hasFnAttribute("wasm-import-module")) {
llvm::AttrBuilder B(M->getContext());
llvm::AttrBuilder B(F->getParent()->getContext());
B.addAttribute("wasm-import-module", "env");
F->addFnAttrs(B);
}
if (!F->hasFnAttribute("wasm-import-name")) {
llvm::AttrBuilder B(M->getContext());
llvm::AttrBuilder B(F->getParent()->getContext());
B.addAttribute("wasm-import-name", F->getName());
F->addFnAttrs(B);
}
return F;
}

// Returns an integer type for the target architecture's address space.
Expand Down Expand Up @@ -493,8 +496,9 @@ WebAssemblyLowerEmscriptenEHSjLj::getFindMatchingCatch(Module &M,
PointerType *Int8PtrTy = PointerType::getUnqual(M.getContext());
SmallVector<Type *, 16> Args(NumClauses, Int8PtrTy);
FunctionType *FTy = FunctionType::get(Int8PtrTy, Args, false);
Function *F = getEmscriptenFunction(
Function *F = getFunction(
FTy, "__cxa_find_matching_catch_" + Twine(NumClauses + 2), &M);
markAsImported(F);
FindMatchingCatches[NumClauses] = F;
return F;
}
Expand Down Expand Up @@ -586,7 +590,8 @@ Function *WebAssemblyLowerEmscriptenEHSjLj::getInvokeWrapper(CallBase *CI) {

FunctionType *FTy = FunctionType::get(CalleeFTy->getReturnType(), ArgTys,
CalleeFTy->isVarArg());
Function *F = getEmscriptenFunction(FTy, "__invoke_" + Sig, M);
Function *F = getFunction(FTy, "__invoke_" + Sig, M);
markAsImported(F);
InvokeWrappers[Sig] = F;
return F;
}
Expand Down Expand Up @@ -927,11 +932,11 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
// exception handling and setjmp/longjmp handling
ThrewGV = getGlobalVariable(M, getAddrIntType(&M), TM, "__THREW__");
ThrewValueGV = getGlobalVariable(M, IRB.getInt32Ty(), TM, "__threwValue");
GetTempRet0F = getEmscriptenFunction(
FunctionType::get(IRB.getInt32Ty(), false), "getTempRet0", &M);
SetTempRet0F = getEmscriptenFunction(
FunctionType::get(IRB.getVoidTy(), IRB.getInt32Ty(), false),
"setTempRet0", &M);
GetTempRet0F = getFunction(FunctionType::get(IRB.getInt32Ty(), false),
"getTempRet0", &M);
SetTempRet0F =
getFunction(FunctionType::get(IRB.getVoidTy(), IRB.getInt32Ty(), false),
"setTempRet0", &M);
GetTempRet0F->setDoesNotThrow();
SetTempRet0F->setDoesNotThrow();

Expand All @@ -942,13 +947,13 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
// Register __resumeException function
FunctionType *ResumeFTy =
FunctionType::get(IRB.getVoidTy(), IRB.getPtrTy(), false);
ResumeF = getEmscriptenFunction(ResumeFTy, "__resumeException", &M);
ResumeF = getFunction(ResumeFTy, "__resumeException", &M);
ResumeF->addFnAttr(Attribute::NoReturn);

// Register llvm_eh_typeid_for function
FunctionType *EHTypeIDTy =
FunctionType::get(IRB.getInt32Ty(), IRB.getPtrTy(), false);
EHTypeIDF = getEmscriptenFunction(EHTypeIDTy, "llvm_eh_typeid_for", &M);
EHTypeIDF = getFunction(EHTypeIDTy, "llvm_eh_typeid_for", &M);
}

// Functions that contains calls to setjmp but don't have other longjmpable
Expand Down Expand Up @@ -988,14 +993,14 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
// Register emscripten_longjmp function
FunctionType *FTy = FunctionType::get(
IRB.getVoidTy(), {getAddrIntType(&M), IRB.getInt32Ty()}, false);
EmLongjmpF = getEmscriptenFunction(FTy, "emscripten_longjmp", &M);
EmLongjmpF = getFunction(FTy, "emscripten_longjmp", &M);
EmLongjmpF->addFnAttr(Attribute::NoReturn);
} else { // EnableWasmSjLj
Type *Int8PtrTy = IRB.getPtrTy();
// Register __wasm_longjmp function, which calls __builtin_wasm_longjmp.
FunctionType *FTy = FunctionType::get(
IRB.getVoidTy(), {Int8PtrTy, IRB.getInt32Ty()}, false);
WasmLongjmpF = getEmscriptenFunction(FTy, "__wasm_longjmp", &M);
WasmLongjmpF = getFunction(FTy, "__wasm_longjmp", &M);
WasmLongjmpF->addFnAttr(Attribute::NoReturn);
}

Expand All @@ -1009,11 +1014,11 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
FunctionType *FTy = FunctionType::get(
IRB.getVoidTy(), {SetjmpFTy->getParamType(0), Int32Ty, Int32PtrTy},
false);
WasmSetjmpF = getEmscriptenFunction(FTy, "__wasm_setjmp", &M);
WasmSetjmpF = getFunction(FTy, "__wasm_setjmp", &M);

// Register __wasm_setjmp_test function
FTy = FunctionType::get(Int32Ty, {Int32PtrTy, Int32PtrTy}, false);
WasmSetjmpTestF = getEmscriptenFunction(FTy, "__wasm_setjmp_test", &M);
WasmSetjmpTestF = getFunction(FTy, "__wasm_setjmp_test", &M);

// wasm.catch() will be lowered down to wasm 'catch' instruction in
// instruction selection.
Expand Down
4 changes: 0 additions & 4 deletions llvm/test/CodeGen/WebAssembly/lower-em-ehsjlj-options.ll
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ target triple = "wasm32-unknown-unknown"
; EH-NOT: .import_name __invoke_void_i32

; SJLJ: .functype emscripten_longjmp (i32, i32) -> ()
; SJLJ: .import_module emscripten_longjmp, env
; SJLJ: .import_name emscripten_longjmp, emscripten_longjmp
; SJLJ-NOT: .functype emscripten_longjmp_jmpbuf
; SJLJ-NOT: .import_module emscripten_longjmp_jmpbuf
; SJLJ-NOT: .import_name emscripten_longjmp_jmpbuf

%struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }

Expand Down
5 changes: 0 additions & 5 deletions llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,7 @@ attributes #0 = { returns_twice }
attributes #1 = { noreturn }
attributes #2 = { nounwind }
attributes #3 = { allocsize(0) }
; CHECK-DAG: attributes #{{[0-9]+}} = { nounwind "wasm-import-module"="env" "wasm-import-name"="getTempRet0" }
; CHECK-DAG: attributes #{{[0-9]+}} = { nounwind "wasm-import-module"="env" "wasm-import-name"="setTempRet0" }
; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__invoke_void" }
; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__wasm_setjmp" }
; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__wasm_setjmp_test" }
; CHECK-DAG: attributes #{{[0-9]+}} = { noreturn "wasm-import-module"="env" "wasm-import-name"="emscripten_longjmp" }
; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__invoke_ptr_i32_ptr" }
; CHECK-DAG: attributes #[[ALLOCSIZE_ATTR]] = { allocsize(1) }

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/WebAssembly/lower-wasm-ehsjlj.ll
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ catch: ; preds = %catch.start
catchret from %2 to label %catchret.dest
; CHECK: catch: ; preds = %catch.start
; CHECK-NEXT: %exn = load ptr, ptr %exn.slot6, align 4
; CHECK-NEXT: %5 = call ptr @__cxa_begin_catch(ptr %exn) #6 [ "funclet"(token %2) ]
; CHECK-NEXT: %5 = call ptr @__cxa_begin_catch(ptr %exn) #3 [ "funclet"(token %2) ]
; CHECK-NEXT: invoke void @__cxa_end_catch() [ "funclet"(token %2) ]
; CHECK-NEXT: to label %.noexc unwind label %catch.dispatch.longjmp

Expand Down