release/19.x: [clang-repl] Fix generation of wasm binaries while running clang-repl in browser (#117978) #118077
Conversation
This file contains 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
|
@vgvassilev What do you think about merging this PR to the release branch? |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (llvmbot) ChangesBackport a174aa1 Requested by: @anutosh491 Full diff: https://github.com/llvm/llvm-project/pull/118077.diff 3 Files Affected:
diff --git a/clang/lib/Interpreter/CMakeLists.txt b/clang/lib/Interpreter/CMakeLists.txt
index 0a2d60757c216d..85efa4b0f984f4 100644
--- a/clang/lib/Interpreter/CMakeLists.txt
+++ b/clang/lib/Interpreter/CMakeLists.txt
@@ -15,6 +15,7 @@ set(LLVM_LINK_COMPONENTS
if (EMSCRIPTEN AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
set(WASM_SRC Wasm.cpp)
set(WASM_LINK lldWasm)
+ set(COMMON_LINK lldCommon)
endif()
add_clang_library(clangInterpreter
@@ -45,6 +46,7 @@ add_clang_library(clangInterpreter
clangSema
clangSerialization
${WASM_LINK}
+ ${COMMON_LINK}
)
if ((MINGW OR CYGWIN) AND BUILD_SHARED_LIBS)
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index c0b8bfebb43437..985d0b7c0ef311 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -193,6 +193,7 @@ IncrementalCompilerBuilder::CreateCpp() {
Argv.push_back("-target");
Argv.push_back("wasm32-unknown-emscripten");
Argv.push_back("-shared");
+ Argv.push_back("-fvisibility=default");
#endif
Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
diff --git a/clang/lib/Interpreter/Wasm.cpp b/clang/lib/Interpreter/Wasm.cpp
index 79efbaa03982d0..aa10b160ccf847 100644
--- a/clang/lib/Interpreter/Wasm.cpp
+++ b/clang/lib/Interpreter/Wasm.cpp
@@ -23,6 +23,31 @@
#include <string>
namespace lld {
+enum Flavor {
+ Invalid,
+ Gnu, // -flavor gnu
+ MinGW, // -flavor gnu MinGW
+ WinLink, // -flavor link
+ Darwin, // -flavor darwin
+ Wasm, // -flavor wasm
+};
+
+using Driver = bool (*)(llvm::ArrayRef<const char *>, llvm::raw_ostream &,
+ llvm::raw_ostream &, bool, bool);
+
+struct DriverDef {
+ Flavor f;
+ Driver d;
+};
+
+struct Result {
+ int retCode;
+ bool canRunAgain;
+};
+
+Result lldMain(llvm::ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
+ llvm::raw_ostream &stderrOS, llvm::ArrayRef<DriverDef> drivers);
+
namespace wasm {
bool link(llvm::ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
llvm::raw_ostream &stderrOS, bool exitEarly, bool disableOutput);
@@ -51,13 +76,14 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
llvm::TargetMachine *TargetMachine = Target->createTargetMachine(
PTU.TheModule->getTargetTriple(), "", "", TO, llvm::Reloc::Model::PIC_);
PTU.TheModule->setDataLayout(TargetMachine->createDataLayout());
- std::string OutputFileName = PTU.TheModule->getName().str() + ".wasm";
+ std::string ObjectFileName = PTU.TheModule->getName().str() + ".o";
+ std::string BinaryFileName = PTU.TheModule->getName().str() + ".wasm";
std::error_code Error;
- llvm::raw_fd_ostream OutputFile(llvm::StringRef(OutputFileName), Error);
+ llvm::raw_fd_ostream ObjectFileOutput(llvm::StringRef(ObjectFileName), Error);
llvm::legacy::PassManager PM;
- if (TargetMachine->addPassesToEmitFile(PM, OutputFile, nullptr,
+ if (TargetMachine->addPassesToEmitFile(PM, ObjectFileOutput, nullptr,
llvm::CodeGenFileType::ObjectFile)) {
return llvm::make_error<llvm::StringError>(
"Wasm backend cannot produce object.", llvm::inconvertibleErrorCode());
@@ -69,27 +95,30 @@ llvm::Error WasmIncrementalExecutor::addModule(PartialTranslationUnit &PTU) {
llvm::inconvertibleErrorCode());
}
- OutputFile.close();
+ ObjectFileOutput.close();
std::vector<const char *> LinkerArgs = {"wasm-ld",
"-shared",
"--import-memory",
- "--no-entry",
- "--export-all",
"--experimental-pic",
"--stack-first",
"--allow-undefined",
- OutputFileName.c_str(),
+ ObjectFileName.c_str(),
"-o",
- OutputFileName.c_str()};
- int Result =
- lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
- if (!Result)
+ BinaryFileName.c_str()};
+
+ const lld::DriverDef WasmDriver = {lld::Flavor::Wasm, &lld::wasm::link};
+ std::vector<lld::DriverDef> WasmDriverArgs;
+ WasmDriverArgs.push_back(WasmDriver);
+ lld::Result Result =
+ lld::lldMain(LinkerArgs, llvm::outs(), llvm::errs(), WasmDriverArgs);
+
+ if (Result.retCode)
return llvm::make_error<llvm::StringError>(
"Failed to link incremental module", llvm::inconvertibleErrorCode());
void *LoadedLibModule =
- dlopen(OutputFileName.c_str(), RTLD_NOW | RTLD_GLOBAL);
+ dlopen(BinaryFileName.c_str(), RTLD_NOW | RTLD_GLOBAL);
if (LoadedLibModule == nullptr) {
llvm::errs() << dlerror() << '\n';
return llvm::make_error<llvm::StringError>(
@@ -109,7 +138,7 @@ llvm::Error WasmIncrementalExecutor::runCtors() const {
return llvm::Error::success();
}
-llvm::Error WasmIncrementalExecutor::cleanUp() const {
+llvm::Error WasmIncrementalExecutor::cleanUp() {
// Can't call cleanUp through IncrementalExecutor as it
// tries to deinitialize JIT which hasn't been initialized
return llvm::Error::success();
@@ -117,4 +146,4 @@ llvm::Error WasmIncrementalExecutor::cleanUp() const {
WasmIncrementalExecutor::~WasmIncrementalExecutor() = default;
-} // namespace clang
+} // namespace clang
\ No newline at end of file
|
vgvassilev
approved these changes
Nov 29, 2024
|
Hi @tru Curious to know as to when a 19.1.5 release is scheduled ? |
Tomorrow. |
… in browser (llvm#117978) Co-authored-by: Vassil Vassilev <[email protected]> (cherry picked from commit a174aa1)
|
@anutosh491 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Fixes addModule function for clang-repl allowing it to be run completely in browser. |
Hi @tru, the above comment requires us to add a label for release notes. Not sure I can add a label. Can you help me out here ? |
Thanks for the update :) |
Ignore this, it should be disabled. |
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.
Backport a174aa1
Requested by: @anutosh491