[clang-repl] Pass triple to IncrementalCompilerBuilder as explicit argument #84174
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Stefan Gränitz (weliveindetail) ChangesWith out-of-process execution the target triple can be different from the one on the host. We need an interface to configure it. Full diff: https://github.com/llvm/llvm-project/pull/84174.diff 3 Files Affected:
diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..8d111aa086867a 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -49,7 +49,7 @@ class IncrementalCompilerBuilder {
}
// General C++
- llvm::Expected<std::unique_ptr<CompilerInstance>> CreateCpp();
+ llvm::Expected<std::unique_ptr<CompilerInstance>> CreateCpp(llvm::Triple TT);
// Offload options
void SetOffloadArch(llvm::StringRef Arch) { OffloadArch = Arch; };
@@ -57,14 +57,17 @@ class IncrementalCompilerBuilder {
// CUDA specific
void SetCudaSDK(llvm::StringRef path) { CudaSDKPath = path; };
- llvm::Expected<std::unique_ptr<CompilerInstance>> CreateCudaHost();
- llvm::Expected<std::unique_ptr<CompilerInstance>> CreateCudaDevice();
+ llvm::Expected<std::unique_ptr<CompilerInstance>>
+ CreateCudaHost(llvm::Triple TT);
+ llvm::Expected<std::unique_ptr<CompilerInstance>>
+ CreateCudaDevice(llvm::Triple TT);
private:
static llvm::Expected<std::unique_ptr<CompilerInstance>>
- create(std::vector<const char *> &ClangArgv);
+ create(llvm::Triple TT, std::vector<const char *> &ClangArgv);
- llvm::Expected<std::unique_ptr<CompilerInstance>> createCuda(bool device);
+ llvm::Expected<std::unique_ptr<CompilerInstance>> createCuda(llvm::Triple TT,
+ bool device);
std::vector<const char *> UserArgs;
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..429c1b65f6665d 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -132,7 +132,8 @@ CreateCI(const llvm::opt::ArgStringList &Argv) {
} // anonymous namespace
llvm::Expected<std::unique_ptr<CompilerInstance>>
-IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {
+IncrementalCompilerBuilder::create(llvm::Triple TT,
+ std::vector<const char *> &ClangArgv) {
// If we don't know ClangArgv0 or the address of main() at this point, try
// to guess it anyway (it's possible on some platforms).
@@ -162,8 +163,7 @@ IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {
TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
- driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0],
- llvm::sys::getProcessTriple(), Diags);
+ driver::Driver Driver(/*MainBinaryName=*/ClangArgv[0], TT.str(), Diags);
Driver.setCheckInputsExist(false); // the input comes from mem buffers
llvm::ArrayRef<const char *> RF = llvm::ArrayRef(ClangArgv);
std::unique_ptr<driver::Compilation> Compilation(Driver.BuildCompilation(RF));
@@ -179,17 +179,17 @@ IncrementalCompilerBuilder::create(std::vector<const char *> &ClangArgv) {
}
llvm::Expected<std::unique_ptr<CompilerInstance>>
-IncrementalCompilerBuilder::CreateCpp() {
+IncrementalCompilerBuilder::CreateCpp(llvm::Triple TT) {
std::vector<const char *> Argv;
Argv.reserve(5 + 1 + UserArgs.size());
Argv.push_back("-xc++");
Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
- return IncrementalCompilerBuilder::create(Argv);
+ return IncrementalCompilerBuilder::create(TT, Argv);
}
llvm::Expected<std::unique_ptr<CompilerInstance>>
-IncrementalCompilerBuilder::createCuda(bool device) {
+IncrementalCompilerBuilder::createCuda(llvm::Triple TT, bool device) {
std::vector<const char *> Argv;
Argv.reserve(5 + 4 + UserArgs.size());
@@ -213,17 +213,17 @@ IncrementalCompilerBuilder::createCuda(bool device) {
Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end());
- return IncrementalCompilerBuilder::create(Argv);
+ return IncrementalCompilerBuilder::create(TT, Argv);
}
llvm::Expected<std::unique_ptr<CompilerInstance>>
-IncrementalCompilerBuilder::CreateCudaDevice() {
- return IncrementalCompilerBuilder::createCuda(true);
+IncrementalCompilerBuilder::CreateCudaDevice(llvm::Triple TT) {
+ return IncrementalCompilerBuilder::createCuda(TT, true);
}
llvm::Expected<std::unique_ptr<CompilerInstance>>
-IncrementalCompilerBuilder::CreateCudaHost() {
- return IncrementalCompilerBuilder::createCuda(false);
+IncrementalCompilerBuilder::CreateCudaHost(llvm::Triple TT) {
+ return IncrementalCompilerBuilder::createCuda(TT, false);
}
Interpreter::Interpreter(std::unique_ptr<CompilerInstance> CI,
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index 5bad8145324d06..96337ada043a6f 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -24,6 +24,7 @@
#include "llvm/Support/ManagedStatic.h" // llvm_shutdown
#include "llvm/Support/Signals.h"
#include "llvm/Support/TargetSelect.h"
+#include "llvm/TargetParser/Host.h"
#include <optional>
// Disable LSan for this test.
@@ -109,7 +110,8 @@ ReplListCompleter::operator()(llvm::StringRef Buffer, size_t Pos,
std::vector<llvm::LineEditor::Completion> Comps;
std::vector<std::string> Results;
- auto CI = CB.CreateCpp();
+ llvm::Triple TT(MainInterp.getCompilerInstance()->getTargetOpts().Triple);
+ auto CI = CB.CreateCpp(TT);
if (auto Err = CI.takeError()) {
ErrRes = std::move(Err);
return {};
@@ -167,6 +169,8 @@ int main(int argc, const char **argv) {
clang::IncrementalCompilerBuilder CB;
CB.SetCompilerArgs(ClangArgv);
+ llvm::Triple TT(llvm::sys::getProcessTriple());
+
std::unique_ptr<clang::CompilerInstance> DeviceCI;
if (CudaEnabled) {
if (!CudaPath.empty())
@@ -177,16 +181,16 @@ int main(int argc, const char **argv) {
}
CB.SetOffloadArch(OffloadArch);
- DeviceCI = ExitOnErr(CB.CreateCudaDevice());
+ DeviceCI = ExitOnErr(CB.CreateCudaDevice(TT));
}
// FIXME: Investigate if we could use runToolOnCodeWithArgs from tooling. It
// can replace the boilerplate code for creation of the compiler instance.
std::unique_ptr<clang::CompilerInstance> CI;
if (CudaEnabled) {
- CI = ExitOnErr(CB.CreateCudaHost());
+ CI = ExitOnErr(CB.CreateCudaHost(TT));
} else {
- CI = ExitOnErr(CB.CreateCpp());
+ CI = ExitOnErr(CB.CreateCpp(TT));
}
// Set an error handler, so that any LLVM backend diagnostics go through our
|
vgvassilev
reviewed
Mar 6, 2024
…gument With out-of-process execution the target triple can be different from the one on the host. We need an interface to configure it.
weliveindetail
force-pushed
the
clang-repl-triple-arg
branch
from
767fccd to
0d7f6a8
Compare
weliveindetail
commented
Mar 6, 2024
| @@ -213,7 +214,8 @@ IncrementalCompilerBuilder::createCuda(bool device) { | |||
|
|
|||
| Argv.insert(Argv.end(), UserArgs.begin(), UserArgs.end()); | |||
|
|
|||
| return IncrementalCompilerBuilder::create(Argv); | |||
| std::string TT = TargetTriple ? *TargetTriple : llvm::sys::getProcessTriple(); | |||
| return IncrementalCompilerBuilder::create(TT, Argv); | |||
There was a problem hiding this comment.
The duplication is due to create() being static right now, but I think it's acceptable.
| @@ -48,6 +48,8 @@ class IncrementalCompilerBuilder { | |||
| UserArgs = Args; | |||
| } | |||
|
|
|||
| void SetTargetTriple(std::string TT) { TargetTriple = TT; } | |||
There was a problem hiding this comment.
That's a lot less intrusive!
|
Thanks for the quick feedback and review! |
|
This triggers the leak detector in our HWASan build bot https://lab.llvm.org/buildbot/#/builders/236/builds/9873/steps/10/logs/stdio |
|
Interesting, thanks for reporting! It's this code: Apparently, it is related to releasing the MemBuffer and passing the raw pointer to |
weliveindetail
added a commit
that referenced
this pull request
Mar 7, 2024
…ilder (#84174) With out-of-process execution the target triple can be different from the one on the host. We need an interface to configure it. Relanding this with cleanup-fixes in the unittest.
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.
With out-of-process execution the target triple can be different from the one on the host. We need an interface to configure it.