[vm] Decouple bytecode reading from compilation
This way requests to compile won't end prematurely after reading
bytecode if interpreter is enabled.
Change-Id: Ic06cae98ea65bc4656ef491fbd306d34b0e1ef4c
Reviewed-on: https://dart-review.googlesource.com/75628
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc
index bac50ed..c68b03f 100644
--- a/runtime/vm/compiler/frontend/bytecode_reader.cc
+++ b/runtime/vm/compiler/frontend/bytecode_reader.cc
@@ -10,6 +10,7 @@
#include "vm/compiler/assembler/disassembler_kbc.h"
#include "vm/constants_kbc.h"
#include "vm/dart_entry.h"
+#include "vm/longjump.h"
#include "vm/object_store.h"
#include "vm/timeline.h"
@@ -711,6 +712,57 @@
return NativeEntryData::New(kind, trampoline, native_function, argc_tag);
}
+RawError* BytecodeReader::ReadFunctionBytecode(Thread* thread,
+ const Function& function) {
+ ASSERT(!FLAG_precompiled_mode);
+ ASSERT(!function.HasBytecode());
+ ASSERT(thread->sticky_error() == Error::null());
+
+ LongJumpScope jump;
+ if (setjmp(*jump.Set()) == 0) {
+ StackZone stack_zone(thread);
+ Zone* const zone = stack_zone.GetZone();
+ HANDLESCOPE(thread);
+ CompilerState compiler_state(thread);
+
+ const Script& script = Script::Handle(zone, function.script());
+ TranslationHelper translation_helper(thread);
+ translation_helper.InitFromScript(script);
+
+ KernelReaderHelper reader_helper(
+ zone, &translation_helper, script,
+ ExternalTypedData::Handle(zone, function.KernelData()),
+ function.KernelDataProgramOffset());
+ ActiveClass active_class;
+ TypeTranslator type_translator(&reader_helper, &active_class,
+ /* finalize= */ true);
+
+ BytecodeMetadataHelper bytecode_metadata_helper(
+ &reader_helper, &type_translator, &active_class);
+
+ // Setup a [ActiveClassScope] and a [ActiveMemberScope] which will be used
+ // e.g. for type translation.
+ const Class& klass = Class::Handle(zone, function.Owner());
+ Function& outermost_function =
+ Function::Handle(zone, function.GetOutermostFunction());
+
+ ActiveClassScope active_class_scope(&active_class, &klass);
+ ActiveMemberScope active_member(&active_class, &outermost_function);
+ ActiveTypeParametersScope active_type_params(&active_class, function, zone);
+
+ bytecode_metadata_helper.ReadMetadata(function);
+
+ return Error::null();
+ } else {
+ StackZone stack_zone(thread);
+ Error& error = Error::Handle();
+ // We got an error during bytecode reading.
+ error = thread->sticky_error();
+ thread->clear_sticky_error();
+ return error.raw();
+ }
+}
+
} // namespace kernel
} // namespace dart
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.h b/runtime/vm/compiler/frontend/bytecode_reader.h
index 769ad61..1ea8c46 100644
--- a/runtime/vm/compiler/frontend/bytecode_reader.h
+++ b/runtime/vm/compiler/frontend/bytecode_reader.h
@@ -41,6 +41,14 @@
DISALLOW_COPY_AND_ASSIGN(BytecodeMetadataHelper);
};
+class BytecodeReader : public AllStatic {
+ public:
+ // Reads bytecode for the given function and sets its bytecode field.
+ // Returns error (if any), or null.
+ static RawError* ReadFunctionBytecode(Thread* thread,
+ const Function& function);
+};
+
} // namespace kernel
} // namespace dart
diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
index e8b948c..5f0e767 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -9,6 +9,7 @@
#include "vm/compiler/frontend/flow_graph_builder.h" // For dart::FlowGraphBuilder::SimpleInstanceOfType.
#include "vm/compiler/frontend/prologue_builder.h"
#include "vm/compiler/jit/compiler.h"
+#include "vm/kernel.h" // For IsFieldInitializer.
#include "vm/object_store.h"
#include "vm/stack_frame.h"
@@ -1908,43 +1909,17 @@
SetOffset(kernel_offset);
- if (FLAG_enable_interpreter || FLAG_use_bytecode_compiler) {
- // TODO(regis): For now, we skip bytecode loading for functions that were
- // synthesized and that do not have bytecode. Since they inherited the
- // kernel offset of a concrete function, the wrong bytecode would be loaded.
- switch (function.kind()) {
- case RawFunction::kImplicitGetter:
- case RawFunction::kImplicitSetter:
- case RawFunction::kMethodExtractor:
- case RawFunction::kNoSuchMethodDispatcher:
- case RawFunction::kInvokeFieldDispatcher:
- case RawFunction::kDynamicInvocationForwarder:
- case RawFunction::kImplicitClosureFunction:
- break;
- case RawFunction::kImplicitStaticFinalGetter:
- if (!IsFieldInitializer(function, Z)) {
- break;
- }
- // Fallthrough.
- default: {
- // TODO(regis): Clean up this logic of when to compile.
- // If the bytecode was previously loaded, we really want to compile.
- if (!function.HasBytecode()) {
- bytecode_metadata_helper_.ReadMetadata(function);
- }
- if (function.HasBytecode()) {
- if (FLAG_use_bytecode_compiler) {
- BytecodeFlowGraphBuilder bytecode_compiler(
- flow_graph_builder_, parsed_function(),
- &(flow_graph_builder_->ic_data_array_));
- FlowGraph* flow_graph = bytecode_compiler.BuildGraph();
- if (flow_graph != nullptr) {
- return flow_graph;
- }
- } else {
- return nullptr;
- }
- }
+ if (FLAG_use_bytecode_compiler && function.IsBytecodeAllowed(Z)) {
+ if (!function.HasBytecode()) {
+ bytecode_metadata_helper_.ReadMetadata(function);
+ }
+ if (function.HasBytecode()) {
+ BytecodeFlowGraphBuilder bytecode_compiler(
+ flow_graph_builder_, parsed_function(),
+ &(flow_graph_builder_->ic_data_array_));
+ FlowGraph* flow_graph = bytecode_compiler.BuildGraph();
+ if (flow_graph != nullptr) {
+ return flow_graph;
}
}
}
diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc
index da11764..a6f15f9 100644
--- a/runtime/vm/compiler/frontend/scope_builder.cc
+++ b/runtime/vm/compiler/frontend/scope_builder.cc
@@ -5,6 +5,7 @@
#include "vm/compiler/frontend/scope_builder.h"
#include "vm/compiler/backend/il.h" // For CompileType.
+#include "vm/kernel.h" // For IsFieldInitializer.
#if !defined(DART_PRECOMPILED_RUNTIME)
@@ -16,12 +17,6 @@
#define T (type_translator_)
#define I Isolate::Current()
-bool IsFieldInitializer(const Function& function, Zone* zone) {
- return (function.kind() == RawFunction::kImplicitStaticFinalGetter) &&
- String::Handle(zone, function.name())
- .StartsWith(Symbols::InitPrefix());
-}
-
// Returns true if the given method can skip type checks for all arguments
// that are not covariant or generic covariant in its implementation.
bool MethodCanSkipTypeChecksForNonCovariantArguments(
diff --git a/runtime/vm/compiler/frontend/scope_builder.h b/runtime/vm/compiler/frontend/scope_builder.h
index 210c51b..12521be 100644
--- a/runtime/vm/compiler/frontend/scope_builder.h
+++ b/runtime/vm/compiler/frontend/scope_builder.h
@@ -228,8 +228,6 @@
DISALLOW_COPY_AND_ASSIGN(ScopeBuildingResult);
};
-bool IsFieldInitializer(const Function& function, Zone* zone);
-
// Returns true if the given method can skip type checks for all arguments
// that are not covariant or generic covariant in its implementation.
bool MethodCanSkipTypeChecksForNonCovariantArguments(
diff --git a/runtime/vm/compiler/jit/compiler.cc b/runtime/vm/compiler/jit/compiler.cc
index 50ff623..8c9eff9 100644
--- a/runtime/vm/compiler/jit/compiler.cc
+++ b/runtime/vm/compiler/jit/compiler.cc
@@ -24,6 +24,7 @@
#include "vm/compiler/cha.h"
#include "vm/compiler/compiler_pass.h"
#include "vm/compiler/compiler_state.h"
+#include "vm/compiler/frontend/bytecode_reader.h"
#include "vm/compiler/frontend/flow_graph_builder.h"
#include "vm/compiler/frontend/kernel_to_il.h"
#include "vm/compiler/jit/jit_call_specializer.h"
@@ -164,8 +165,7 @@
/* not inlining */ NULL, optimized,
osr_id);
FlowGraph* graph = builder.BuildGraph();
- ASSERT((graph != NULL) || (FLAG_enable_interpreter &&
- parsed_function->function().HasBytecode()));
+ ASSERT(graph != NULL);
return graph;
}
FlowGraphBuilder builder(*parsed_function, *ic_data_array,
@@ -249,9 +249,25 @@
// Arg0: function object.
DEFINE_RUNTIME_ENTRY(CompileFunction, 1) {
const Function& function = Function::CheckedHandle(arguments.ArgAt(0));
+ Object& result = Object::Handle(zone);
ASSERT(!function.HasCode());
- const Object& result =
- Object::Handle(Compiler::CompileFunction(thread, function));
+
+ if (FLAG_enable_interpreter && function.IsBytecodeAllowed(zone)) {
+ if (!function.HasBytecode()) {
+ result = kernel::BytecodeReader::ReadFunctionBytecode(thread, function);
+ if (!result.IsNull()) {
+ Exceptions::PropagateError(Error::Cast(result));
+ }
+ }
+ if (function.HasBytecode()) {
+ // Verify that InterpretCall stub code was installed.
+ ASSERT(function.CurrentCode() == StubCode::InterpretCall_entry()->code());
+ return;
+ }
+ // No bytecode, fall back to compilation.
+ }
+
+ result = Compiler::CompileFunction(thread, function);
if (result.IsError()) {
if (result.IsLanguageError()) {
Exceptions::ThrowCompileTimeError(LanguageError::Cast(result));
@@ -259,13 +275,6 @@
}
Exceptions::PropagateError(Error::Cast(result));
}
- // TODO(regis): Revisit.
- if (FLAG_enable_interpreter && !function.HasCode() &&
- function.HasBytecode()) {
- // Function was not actually compiled, but its bytecode was loaded.
- // Verify that InterpretCall stub code was installed.
- ASSERT(function.CurrentCode() == StubCode::InterpretCall_entry()->code());
- }
}
bool Compiler::CanOptimizeFunction(Thread* thread, const Function& function) {
@@ -830,12 +839,6 @@
zone, parsed_function(), ic_data_array, osr_id(), optimized());
}
- // TODO(regis): Revisit.
- if (FLAG_enable_interpreter && (flow_graph == NULL) &&
- function.HasBytecode()) {
- return Code::null();
- }
-
const bool print_flow_graph =
(FLAG_print_flow_graph ||
(optimized() && FLAG_print_flow_graph_optimized)) &&
@@ -1019,11 +1022,6 @@
const Code& result = Code::Handle(helper.Compile(pipeline));
- // TODO(regis): Revisit.
- if (FLAG_enable_interpreter && result.IsNull() && function.HasBytecode()) {
- return Object::null();
- }
-
if (!result.IsNull()) {
if (!optimized) {
function.SetWasCompiled(true);
@@ -1254,10 +1252,6 @@
RawError* Compiler::EnsureUnoptimizedCode(Thread* thread,
const Function& function) {
- if (FLAG_enable_interpreter && function.HasBytecode()) {
- // TODO(regis): This may not be sufficient when deoptimizing. Revisit then.
- return Error::null();
- }
if (function.unoptimized_code() != Object::null()) {
return Error::null();
}
@@ -1273,9 +1267,6 @@
if (result.IsError()) {
return Error::Cast(result).raw();
}
- if (FLAG_enable_interpreter && function.HasBytecode()) {
- return Error::null();
- }
// Since CompileFunctionHelper replaces the current code, re-attach the
// the original code if the function was already compiled.
if (!original_code.IsNull() && result.raw() == function.CurrentCode() &&
@@ -1411,9 +1402,6 @@
func ^= functions.At(i);
ASSERT(!func.IsNull());
if (!func.HasCode() &&
- // TODO(regis): Revisit.
- // Do not compile function if its bytecode is already loaded.
- (!FLAG_enable_interpreter || !func.HasBytecode()) &&
!func.is_abstract() && !func.IsRedirectingFactory()) {
if ((cls.is_mixin_app_alias() || cls.IsMixinApplication()) &&
func.HasOptionalParameters()) {
@@ -1424,10 +1412,7 @@
if (result.IsError()) {
return Error::Cast(result).raw();
}
- // TODO(regis): Revisit.
- // The compiler may load bytecode and return Code::null().
- ASSERT(!result.IsNull() ||
- (FLAG_enable_interpreter && func.HasBytecode()));
+ ASSERT(!result.IsNull());
}
}
return Error::null();
@@ -1508,20 +1493,30 @@
parsed_function = Parser::ParseStaticFieldInitializer(field);
parsed_function->AllocateVariables();
}
+ const Function& initializer = parsed_function->function();
+
+ if (FLAG_enable_interpreter) {
+ ASSERT(initializer.IsBytecodeAllowed(zone));
+ if (!initializer.HasBytecode()) {
+ RawError* error =
+ kernel::BytecodeReader::ReadFunctionBytecode(thread, initializer);
+ if (error != Error::null()) {
+ return error;
+ }
+ }
+ if (initializer.HasBytecode()) {
+ return DartEntry::InvokeFunction(initializer, Object::empty_array());
+ }
+ }
// Non-optimized code generator.
DartCompilationPipeline pipeline;
CompileParsedFunctionHelper helper(parsed_function, false, kNoOSRDeoptId);
const Code& code = Code::Handle(helper.Compile(&pipeline));
- const Function& initializer = parsed_function->function();
if (!code.IsNull()) {
code.set_var_descriptors(Object::empty_var_descriptors());
return DartEntry::InvokeFunction(initializer, Object::empty_array());
- } else if (FLAG_enable_interpreter && initializer.HasBytecode()) {
- // In case the initializer has bytecode, the compilation step above only
- // loaded the bytecode without generating code.
- return DartEntry::InvokeFunction(initializer, Object::empty_array());
}
}
}
@@ -1770,8 +1765,14 @@
while (running_ && !function.IsNull() && !isolate_->IsTopLevelParsing()) {
// Check that we have aggregated and cleared the stats.
ASSERT(thread->compiler_stats()->IsCleared());
- Compiler::CompileOptimizedFunction(thread, function,
- Compiler::kNoOSRDeoptId);
+ // If running with interpreter, do the unoptimized compilation first.
+ if (FLAG_enable_interpreter &&
+ (function.unoptimized_code() == Code::null())) {
+ Compiler::EnsureUnoptimizedCode(thread, function);
+ } else {
+ Compiler::CompileOptimizedFunction(thread, function,
+ Compiler::kNoOSRDeoptId);
+ }
#ifndef PRODUCT
Isolate* isolate = thread->isolate();
isolate->aggregate_compiler_stats()->Add(*thread->compiler_stats());
diff --git a/runtime/vm/dart_entry.cc b/runtime/vm/dart_entry.cc
index a49749d..5708a0d 100644
--- a/runtime/vm/dart_entry.cc
+++ b/runtime/vm/dart_entry.cc
@@ -6,6 +6,7 @@
#include "platform/safe_stack.h"
#include "vm/class_finalizer.h"
+#include "vm/compiler/frontend/bytecode_reader.h"
#include "vm/compiler/jit/compiler.h"
#include "vm/debugger.h"
#include "vm/heap/safepoint.h"
@@ -130,29 +131,35 @@
ScopedIsolateStackLimits stack_limit(thread, current_sp);
#if !defined(DART_PRECOMPILED_RUNTIME)
if (!function.HasCode()) {
- // There's no native code. If we're not using the interpreter, then we
- // compile to native code. If we are using the interpreter, but there's no
- // native code and no bytecode, then we invoke the compiler to extract the
- // bytecode.
- if (!FLAG_enable_interpreter || !function.HasBytecode()) {
- const Object& result =
- Object::Handle(zone, Compiler::CompileFunction(thread, function));
- if (result.IsError()) {
- return Error::Cast(result).raw();
+ if (FLAG_enable_interpreter && function.IsBytecodeAllowed(zone)) {
+ if (!function.HasBytecode()) {
+ RawError* error =
+ kernel::BytecodeReader::ReadFunctionBytecode(thread, function);
+ if (error != Error::null()) {
+ return error;
+ }
}
+
+ // If we have bytecode but no native code then invoke the interpreter.
+ if (function.HasBytecode()) {
+ ASSERT(thread->no_callback_scope_depth() == 0);
+ SuspendLongJumpScope suspend_long_jump_scope(thread);
+ TransitionToGenerated transition(thread);
+ return Interpreter::Current()->Call(function, arguments_descriptor,
+ arguments, thread);
+ }
+
+ // No bytecode, fall back to compilation.
}
- // At this point we should have either native code or bytecode.
- ASSERT(function.HasCode() || function.HasBytecode());
-
- // If we have bytecode but no native code then invoke the interpreter.
- if (!function.HasCode() && function.HasBytecode()) {
- ASSERT(thread->no_callback_scope_depth() == 0);
- SuspendLongJumpScope suspend_long_jump_scope(thread);
- TransitionToGenerated transition(thread);
- return Interpreter::Current()->Call(function, arguments_descriptor,
- arguments, thread);
+ const Object& result =
+ Object::Handle(zone, Compiler::CompileFunction(thread, function));
+ if (result.IsError()) {
+ return Error::Cast(result).raw();
}
+
+ // At this point we should have native code.
+ ASSERT(function.HasCode());
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
index 30f52a8..6827707 100644
--- a/runtime/vm/debugger.cc
+++ b/runtime/vm/debugger.cc
@@ -319,6 +319,8 @@
// Extract the function and the code from the asynchronous activation.
function_ = async_activation.function();
#if !defined(DART_PRECOMPILED_RUNTIME)
+ // TODO(regis): Revise debugger functionality when running a mix of
+ // interpreted and compiled code.
if (!FLAG_enable_interpreter || !function_.HasBytecode()) {
function_.EnsureHasCompiledUnoptimizedCode();
}
diff --git a/runtime/vm/kernel.cc b/runtime/vm/kernel.cc
index 7bbc8f8..335a140 100644
--- a/runtime/vm/kernel.cc
+++ b/runtime/vm/kernel.cc
@@ -686,6 +686,12 @@
return false;
}
+bool IsFieldInitializer(const Function& function, Zone* zone) {
+ return (function.kind() == RawFunction::kImplicitStaticFinalGetter) &&
+ String::Handle(zone, function.name())
+ .StartsWith(Symbols::InitPrefix());
+}
+
} // namespace kernel
} // namespace dart
diff --git a/runtime/vm/kernel.h b/runtime/vm/kernel.h
index 5b8e9a2..534d18f 100644
--- a/runtime/vm/kernel.h
+++ b/runtime/vm/kernel.h
@@ -198,6 +198,8 @@
// as such function already checks all of its parameters.
bool NeedsDynamicInvocationForwarder(const Function& function);
+bool IsFieldInitializer(const Function& function, Zone* zone);
+
} // namespace kernel
} // namespace dart
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index be89708..256aaae 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -5966,6 +5966,24 @@
}
#if !defined(DART_PRECOMPILED_RUNTIME)
+bool Function::IsBytecodeAllowed(Zone* zone) const {
+ switch (kind()) {
+ case RawFunction::kImplicitGetter:
+ case RawFunction::kImplicitSetter:
+ case RawFunction::kMethodExtractor:
+ case RawFunction::kNoSuchMethodDispatcher:
+ case RawFunction::kInvokeFieldDispatcher:
+ case RawFunction::kDynamicInvocationForwarder:
+ case RawFunction::kImplicitClosureFunction:
+ case RawFunction::kIrregexpFunction:
+ return false;
+ case RawFunction::kImplicitStaticFinalGetter:
+ return kernel::IsFieldInitializer(*this, zone);
+ default:
+ return true;
+ }
+}
+
void Function::AttachBytecode(const Code& value) const {
DEBUG_ASSERT(IsMutatorOrAtSafepoint());
// Finish setting up code before activating it.
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index 11ea398..8d6ad6a 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -2376,6 +2376,7 @@
}
#if !defined(DART_PRECOMPILED_RUNTIME)
+ bool IsBytecodeAllowed(Zone* zone) const;
void AttachBytecode(const Code& bytecode) const;
RawCode* Bytecode() const { return raw_ptr()->bytecode_; }
bool HasBytecode() const;
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index eca3f35..330f368 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -7,6 +7,7 @@
#include "vm/ast.h"
#include "vm/code_patcher.h"
#include "vm/compiler/assembler/assembler.h"
+#include "vm/compiler/frontend/bytecode_reader.h"
#include "vm/compiler/jit/compiler.h"
#include "vm/dart_api_impl.h"
#include "vm/dart_entry.h"
