[Lldb-commits] [lldb] [lldb][ClangExpressionParser][NFC] Factor out TargetOpts logic out of ClangExpressionParser (PR #101681)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 2 07:35:06 PDT 2024


https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/101681

Same motivation as https://github.com/llvm/llvm-project/pull/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

>From 904f5938946bd73eb06884c139b6c75446a9ce88 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 2 Aug 2024 15:27:16 +0100
Subject: [PATCH] [lldb][ClangExpressionParser][NFC] Factor out TargetOpts
 logic out of ClangExpressionParser

---
 .../Clang/ClangExpressionParser.cpp           | 142 +++++++++---------
 .../Clang/ClangExpressionParser.h             |   9 --
 2 files changed, 75 insertions(+), 76 deletions(-)

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.
   ///



More information about the lldb-commits mailing list