ldrumm updated this revision to Diff 45588.
ldrumm added a comment.
Address @spyffe's previous suggestions regarding modifying target options in
place. This allows for a simpler and more maintainable solution than previously
implemented, and requires no extra datatype definitions.
- removed unneccessary SetOverrideTargetOpts method.
- removed unused `OverrideExprOptions` type.
- GetOverrideExprOpts now takes a single clang::TargetOptions reference
argument which is modified in-place, and returns a bool value indicating
whether the options were modified by the runtime.
http://reviews.llvm.org/D15527
Files:
include/lldb/Target/LanguageRuntime.h
source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -173,7 +173,7 @@
// 1. Create a new compiler instance.
m_compiler.reset(new CompilerInstance());
lldb::LanguageType frame_lang = expr.Language(); // defaults to lldb::eLanguageTypeUnknown
- lldb_private::LanguageRuntime::OverrideExprOptions *target_opts_override = nullptr;
+ bool overridden_target_opts = false;
lldb_private::LanguageRuntime *lang_rt = nullptr;
lldb::TargetSP target_sp;
if (exe_scope)
@@ -195,83 +195,65 @@
log->Printf("Frame has language of type %s", Language::GetNameForLanguageType(frame_lang));
}
- // 2. Configure the target, overriding with any custom options we can get.
- if (lang_rt)
+ // 2. Configure the compiler with a set of default options that are appropriate
+ // for most situations.
+ if (target_sp && target_sp->GetArchitecture().IsValid())
{
- // The expression is being evaluated in a known LanguageRuntime and StackFrame.
- // We check if there are any overridden options for the evaluation, and configure
- // the compiler appropriately.
- target_opts_override = lang_rt->GetOverrideExprOptions();
+ std::string triple = target_sp->GetArchitecture().GetTriple().str();
+ m_compiler->getTargetOpts().Triple = triple;
+ if (log)
+ log->Printf("Using %s as the target triple", m_compiler->getTargetOpts().Triple.c_str());
}
-
- if (target_opts_override)
+ 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();
if (log)
- log->Debug("Using overridden target options for the expression evaluation");
-
- // If we get here this means the given language runtime has a custom set of code
- // generation requirements, and they are configured from the language runtime.
- m_compiler->getTargetOpts().Features = target_opts_override->Features;
- m_compiler->getTargetOpts().CPU = target_opts_override->CPU;
- m_compiler->getTargetOpts().Triple = target_opts_override->Triple;
- m_compiler->getTargetOpts().FeaturesAsWritten = target_opts_override->FeaturesAsWritten;
- m_compiler->getTargetOpts().Reciprocals = target_opts_override->Reciprocals;
+ log->Printf("Using default target triple of %s", m_compiler->getTargetOpts().Triple.c_str());
}
- else
+ // 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)
{
- // In this case, a specialized language runtime is not available and
- // we have to fallback to making some basic assumptions on the target architecture.
- // For 99% of use cases, this will be fine and is assumed to be the default.
- if (target_sp && target_sp->GetArchitecture().IsValid())
- {
- std::string triple = target_sp->GetArchitecture().GetTriple().str();
- m_compiler->getTargetOpts().Triple = triple;
- if (log)
- log->Printf("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 rather than letting this default path execute.
- m_compiler->getTargetOpts().Triple = llvm::sys::getDefaultTargetTriple();
- if (log)
- log->Printf("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_sp->GetArchitecture().GetMachine() == llvm::Triple::x86 ||
- target_sp->GetArchitecture().GetMachine() == llvm::Triple::x86_64)
- {
- m_compiler->getTargetOpts().Features.push_back("+sse");
- m_compiler->getTargetOpts().Features.push_back("+sse2");
- }
+ m_compiler->getTargetOpts().ABI = "apcs-gnu";
}
-
- if (log)
+ // Supported subsets of x86
+ if (target_sp->GetArchitecture().GetMachine() == llvm::Triple::x86 ||
+ target_sp->GetArchitecture().GetMachine() == llvm::Triple::x86_64)
{
- auto opts = m_compiler->getTargetOpts();
- log->Debug("Triple: '%s'", opts.Triple.c_str());
- log->Debug("CPU: '%s'", opts.CPU.c_str());
- log->Debug("FPMath: '%s'", opts.FPMath.c_str());
- log->Debug("ABI: '%s'", opts.ABI.c_str());
- log->Debug("LinkerVersion: '%s'", opts.LinkerVersion.c_str());
- StringList::LogDump(log, opts.FeaturesAsWritten, "FeaturesAsWritten");
- StringList::LogDump(log, opts.Features, "Features");
- StringList::LogDump(log, opts.Reciprocals, "Reciprocals");
+ m_compiler->getTargetOpts().Features.push_back("+sse");
+ m_compiler->getTargetOpts().Features.push_back("+sse2");
}
- // 3. Create and install the target on the compiler.
+ // 3. Now allow the runtime to provide custom configuration options for the target.
+ // In this case, a specialized language runtime is available and we can query it for extra options.
+ // For 99% of use cases, this will not be needed and should be provided when basic platform detection is not enough.
+ if (lang_rt)
+ overridden_target_opts = lang_rt->GetOverrideExprOptions(m_compiler->getTargetOpts());
+
+ if (overridden_target_opts)
+ if (log)
+ {
+ log->Debug("Using overridden target options for the expression evaluation");
+
+ auto opts = m_compiler->getTargetOpts();
+ log->Debug("Triple: '%s'", opts.Triple.c_str());
+ log->Debug("CPU: '%s'", opts.CPU.c_str());
+ log->Debug("FPMath: '%s'", opts.FPMath.c_str());
+ log->Debug("ABI: '%s'", opts.ABI.c_str());
+ log->Debug("LinkerVersion: '%s'", opts.LinkerVersion.c_str());
+ StringList::LogDump(log, opts.FeaturesAsWritten, "FeaturesAsWritten");
+ StringList::LogDump(log, opts.Features, "Features");
+ StringList::LogDump(log, opts.Reciprocals, "Reciprocals");
+ }
+
+ // 4. Create and install the target on the compiler.
m_compiler->createDiagnostics();
auto target_info = TargetInfo::CreateTargetInfo(m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
if (log)
@@ -285,8 +267,7 @@
assert (m_compiler->hasTarget());
- // 4. Set language options.
-
+ // 5. Set language options.
lldb::LanguageType language = expr.Language();
switch (language)
@@ -393,11 +374,11 @@
// created. This complexity should be lifted elsewhere.
m_compiler->getTarget().adjust(m_compiler->getLangOpts());
- // 5. Set up the diagnostic buffer for reporting errors
+ // 6. Set up the diagnostic buffer for reporting errors
m_compiler->getDiagnostics().setClient(new clang::TextDiagnosticBuffer);
- // 6. Set up the source management objects inside the compiler
+ // 7. Set up the source management objects inside the compiler
clang::FileSystemOptions file_system_options;
m_file_manager.reset(new clang::FileManager(file_system_options));
@@ -416,7 +397,7 @@
m_compiler->getPreprocessor().addPPCallbacks(std::move(pp_callbacks));
}
- // 7. Most of this we get from the CompilerInstance, but we
+ // 8. Most of this we get from the CompilerInstance, but we
// also want to give the context an ExternalASTSource.
m_selector_table.reset(new SelectorTable());
m_builtin_context.reset(new Builtin::Context());
Index: include/lldb/Target/LanguageRuntime.h
===================================================================
--- include/lldb/Target/LanguageRuntime.h
+++ include/lldb/Target/LanguageRuntime.h
@@ -31,7 +31,6 @@
{
public:
- class OverrideExprOptions : public clang::TargetOptions {};
~LanguageRuntime() override;
static LanguageRuntime*
@@ -149,14 +148,11 @@
{
}
- virtual OverrideExprOptions *
- GetOverrideExprOptions()
- {
- return nullptr;
- }
-
+ // Called by the Clang expression evaluation engine to allow runtimes to alter the set of target options provided to
+ // the compiler.
+ // If the options prototype is modified, runtimes must return true, false otherwise.
virtual bool
- SetOverrideExprOptions(OverrideExprOptions *opts)
+ GetOverrideExprOptions(clang::TargetOptions &prototype)
{
return false;
}
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
