-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb][ClangExpressionParser][NFC] Factor out TargetOpts logic out of ClangExpressionParser #101681
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
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
… ClangExpressionParser
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesSame motivation as #101669. We plan to eventually use the Clang driver to initialize the This should make refactorings of this code more straightforward. Changes:
Full diff: https://github.com/llvm/llvm-project/pull/101681.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 303e88feea20b..2d34f657793c3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -351,6 +351,80 @@ static void SetupDefaultClangDiagnostics(CompilerInstance &compiler) {
}
}
+/// Returns a string representing current ABI.
+///
+/// \param[in] target_arch
+/// The target architecture.
+///
+/// \return
+/// A string representing target ABI for the current architecture.
+static std::string GetClangTargetABI(const ArchSpec &target_arch) {
+ std::string abi;
+
+ if (target_arch.IsMIPS()) {
+ switch (target_arch.GetFlags() & ArchSpec::eMIPSABI_mask) {
+ case ArchSpec::eMIPSABI_N64:
+ abi = "n64";
+ break;
+ case ArchSpec::eMIPSABI_N32:
+ abi = "n32";
+ break;
+ case ArchSpec::eMIPSABI_O32:
+ abi = "o32";
+ break;
+ default:
+ break;
+ }
+ }
+ return abi;
+}
+
+static void SetupTargetOpts(CompilerInstance &compiler,
+ lldb_private::Target const &target) {
+ Log *log = GetLog(LLDBLog::Expressions);
+ ArchSpec target_arch = target.GetArchitecture();
+
+ const auto target_machine = target_arch.GetMachine();
+ if (target_arch.IsValid()) {
+ std::string triple = target_arch.GetTriple().str();
+ compiler.getTargetOpts().Triple = triple;
+ LLDB_LOGF(log, "Using %s as the target triple",
+ compiler.getTargetOpts().Triple.c_str());
+ } else {
+ // If we get here we don't have a valid target and just have to guess.
+ // Sometimes this will be ok to just use the host target triple (when we
+ // evaluate say "2+3", but other expressions like breakpoint conditions and
+ // other things that _are_ target specific really shouldn't just be using
+ // the host triple. In such a case the language runtime should expose an
+ // overridden options set (3), below.
+ compiler.getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
+ LLDB_LOGF(log, "Using default target triple of %s",
+ compiler.getTargetOpts().Triple.c_str());
+ }
+ // Now add some special fixes for known architectures: Any arm32 iOS
+ // environment, but not on arm64
+ if (compiler.getTargetOpts().Triple.find("arm64") == std::string::npos &&
+ compiler.getTargetOpts().Triple.find("arm") != std::string::npos &&
+ compiler.getTargetOpts().Triple.find("ios") != std::string::npos) {
+ compiler.getTargetOpts().ABI = "apcs-gnu";
+ }
+ // Supported subsets of x86
+ if (target_machine == llvm::Triple::x86 ||
+ target_machine == llvm::Triple::x86_64) {
+ compiler.getTargetOpts().FeaturesAsWritten.push_back("+sse");
+ compiler.getTargetOpts().FeaturesAsWritten.push_back("+sse2");
+ }
+
+ // Set the target CPU to generate code for. This will be empty for any CPU
+ // that doesn't really need to make a special
+ // CPU string.
+ compiler.getTargetOpts().CPU = target_arch.GetClangTargetCPU();
+
+ // Set the target ABI
+ if (std::string abi = GetClangTargetABI(target_arch); !abi.empty())
+ compiler.getTargetOpts().ABI = std::move(abi);
+}
+
//===----------------------------------------------------------------------===//
// Implementation of ClangExpressionParser
//===----------------------------------------------------------------------===//
@@ -395,12 +469,6 @@ ClangExpressionParser::ClangExpressionParser(
// Defaults to lldb::eLanguageTypeUnknown.
lldb::LanguageType frame_lang = expr.Language().AsLanguageType();
- std::string abi;
- ArchSpec target_arch;
- target_arch = target_sp->GetArchitecture();
-
- const auto target_machine = target_arch.GetMachine();
-
// If the expression is being evaluated in the context of an existing stack
// frame, we introspect to see if the language runtime is available.
@@ -419,45 +487,7 @@ ClangExpressionParser::ClangExpressionParser(
// 2. Configure the compiler with a set of default options that are
// appropriate for most situations.
- if (target_arch.IsValid()) {
- std::string triple = target_arch.GetTriple().str();
- m_compiler->getTargetOpts().Triple = triple;
- LLDB_LOGF(log, "Using %s as the target triple",
- m_compiler->getTargetOpts().Triple.c_str());
- } else {
- // If we get here we don't have a valid target and just have to guess.
- // Sometimes this will be ok to just use the host target triple (when we
- // evaluate say "2+3", but other expressions like breakpoint conditions and
- // other things that _are_ target specific really shouldn't just be using
- // the host triple. In such a case the language runtime should expose an
- // overridden options set (3), below.
- m_compiler->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
- LLDB_LOGF(log, "Using default target triple of %s",
- m_compiler->getTargetOpts().Triple.c_str());
- }
- // Now add some special fixes for known architectures: Any arm32 iOS
- // environment, but not on arm64
- if (m_compiler->getTargetOpts().Triple.find("arm64") == std::string::npos &&
- m_compiler->getTargetOpts().Triple.find("arm") != std::string::npos &&
- m_compiler->getTargetOpts().Triple.find("ios") != std::string::npos) {
- m_compiler->getTargetOpts().ABI = "apcs-gnu";
- }
- // Supported subsets of x86
- if (target_machine == llvm::Triple::x86 ||
- target_machine == llvm::Triple::x86_64) {
- m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse");
- m_compiler->getTargetOpts().FeaturesAsWritten.push_back("+sse2");
- }
-
- // Set the target CPU to generate code for. This will be empty for any CPU
- // that doesn't really need to make a special
- // CPU string.
- m_compiler->getTargetOpts().CPU = target_arch.GetClangTargetCPU();
-
- // Set the target ABI
- abi = GetClangTargetABI(target_arch);
- if (!abi.empty())
- m_compiler->getTargetOpts().ABI = abi;
+ SetupTargetOpts(*m_compiler, *target_sp);
// 3. Create and install the target on the compiler.
m_compiler->createDiagnostics();
@@ -1179,28 +1209,6 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager,
return num_errors;
}
-std::string
-ClangExpressionParser::GetClangTargetABI(const ArchSpec &target_arch) {
- std::string abi;
-
- if (target_arch.IsMIPS()) {
- switch (target_arch.GetFlags() & ArchSpec::eMIPSABI_mask) {
- case ArchSpec::eMIPSABI_N64:
- abi = "n64";
- break;
- case ArchSpec::eMIPSABI_N32:
- abi = "n32";
- break;
- case ArchSpec::eMIPSABI_O32:
- abi = "o32";
- break;
- default:
- break;
- }
- }
- return abi;
-}
-
/// Applies the given Fix-It hint to the given commit.
static void ApplyFixIt(const FixItHint &fixit, clang::edit::Commit &commit) {
// This is cobbed from clang::Rewrite::FixItRewriter.
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
index 0852e928a9d42..93e0b007dbcc8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
@@ -119,15 +119,6 @@ class ClangExpressionParser : public ExpressionParser {
bool &can_interpret,
lldb_private::ExecutionPolicy execution_policy) override;
- /// Returns a string representing current ABI.
- ///
- /// \param[in] target_arch
- /// The target architecture.
- ///
- /// \return
- /// A string representing target ABI for the current architecture.
- std::string GetClangTargetABI(const ArchSpec &target_arch);
-
private:
/// Parses the expression.
///
|
adrian-prantl
pushed a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Oct 11, 2024
… ClangExpressionParser (llvm#101681) Same motivation as llvm#101669. We plan to eventually use the Clang driver to initialize the `CompilerInstance`. This should make refactorings of this code more straightforward. **Changes**: * Introduced `SetupTargetOpts` * Called them from `ClangExpressionParser::ClangExpressionParser` * Made `GetClangTargetABI` a file-local function since it's not using any of the state in `ClangExpressionParser`, and isn't used anywhere outside the source file (cherry picked from commit d6cbcf9)
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.
Same motivation as #101669. We plan to eventually use the Clang driver to initialize the
CompilerInstance
.This should make refactorings of this code more straightforward.
Changes:
SetupTargetOpts
ClangExpressionParser::ClangExpressionParser
GetClangTargetABI
a file-local function since it's not using any of the state inClangExpressionParser
, and isn't used anywhere outside the source file