Skip to content
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.

Sign up for GitHub

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

Revise LLVM fix to work when no V8 or WABT available #7635

Merged
merged 4 commits into from
Jun 15, 2023
Merged

Revise LLVM fix to work when no V8 or WABT available #7635

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
131b5ad
Revise LLVM fix to work when no V8 or WABT available
steven-johnson Jun 14, 2023
5956dbd
Update WasmExecutor.cpp
steven-johnson Jun 14, 2023
44553f2
Update WasmExecutor.cpp
steven-johnson Jun 14, 2023
ec0649e
Update WasmExecutor.cpp
steven-johnson Jun 14, 2023
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
32 changes: 26 additions & 6 deletions src/WasmExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@
#endif // WITH_V8
// clang-format on

#if WITH_WABT || WITH_V8
#if LLVM_VERSION >= 170
LLD_HAS_DRIVER(wasm)
#endif
#endif

namespace Halide {
namespace Internal {
Expand Down Expand Up @@ -339,7 +341,10 @@ std::vector<char> compile_to_wasm(const Module &module, const std::string &fn_na

std::string lld_arg_strs[] = {
"HalideJITLinker",
"-flavor", "wasm",
#if LLVM_VERSION >= 170
"-flavor",
"wasm",
#endif
// For debugging purposes:
// "--verbose",
// "-error-limit=0",
Expand All @@ -350,7 +355,8 @@ std::vector<char> compile_to_wasm(const Module &module, const std::string &fn_na
obj_file.pathname(),
"--entry=" + fn_name,
"-o",
wasm_output.pathname()};
wasm_output.pathname()
};

constexpr int c = sizeof(lld_arg_strs) / sizeof(lld_arg_strs[0]);
const char *lld_args[c];
Expand All @@ -359,15 +365,29 @@ std::vector<char> compile_to_wasm(const Module &module, const std::string &fn_na
}

#if LLVM_VERSION >= 170
// lld will temporarily hijack the signal handlers to ensure that temp files get cleaned up,
// but rather than preserving custom handlers in place, it restores the default handlers.
// This conflicts with some of our testing infrastructure, which relies on a SIGABRT handler
// set at global-ctor time to stay set. Therefore we'll save and restore this ourselves.
// Note that we must restore it before using internal_error (and also on the non-error path).
auto old_abort_handler = std::signal(SIGABRT, SIG_DFL);

llvm::ArrayRef<const char *> args(lld_args, lld_args + c);
auto r = lld::lldMain(args, llvm::outs(), llvm::errs(), {{lld::Wasm, &lld::wasm::link}});
if (!r.canRunAgain) {
std::cerr << "lld::wasm::link failed catastrophically, exiting with: " << r.retCode << "\n";
lld::exitLld(r.retCode); // Exit now, can't re-execute again.
}
// TODO: https://reviews.llvm.org/D119049 suggests that you should call exitLld()
// if canRunAgain is false, but doing do fails with SIGABRT rather than exit(1), which
// breaks our error tests. For now, just following the old practice.
//
// if (!r.canRunAgain) {
// std::cerr << "lld::wasm::link failed catastrophically, exiting with: " << r.retCode << "\n";
// lld::exitLld(r.retCode); // Exit now, can't re-execute again.
// }
if (r.retCode != 0) {
std::signal(SIGABRT, old_abort_handler);
internal_error << "lld::wasm::link failed with: " << r.retCode << "\n";
}

std::signal(SIGABRT, old_abort_handler);
#else
// lld will temporarily hijack the signal handlers to ensure that temp files get cleaned up,
// but rather than preserving custom handlers in place, it restores the default handlers.
Expand Down