Commit 28d2b323 authored by Jakob Gruber's avatar Jakob Gruber Committed by V8 LUCI CQ

[compiler] Fine-grained JSFunctionData validation

JSFunctionData has a fairly heavy serialized payload, and likewise
consistency validation validates many fields and thus has many
opportunities to fail. We therefore want to avoid or reduce validation
whenever possible.

This CL adds tracking s.t. we know which fields were actually used,
and we limit validation to used fields.

Drive-by: Make serialized_ debug-only.
Drive-by: Don't create deps for context/native_context/shared.

Bug: v8:7790
Change-Id: Ic32c9919f0c75a76d9c36e4396b6bce383151b62
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3132962
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76614}
parent 0105be26
This diff is collapsed.
......@@ -459,8 +459,10 @@ class V8_EXPORT_PRIVATE JSFunctionRef : public JSObjectRef {
// the main thread.
bool IsConsistentWithHeapState() const;
// TODO(jgruber): Consider more fine-grained dependencies that keep track of
// which fields were actually inspected during compilation.
ContextRef context() const;
NativeContextRef native_context() const;
SharedFunctionInfoRef shared() const;
bool has_feedback_vector(CompilationDependencies* dependencies) const;
bool has_initial_map(CompilationDependencies* dependencies) const;
bool PrototypeRequiresRuntimeLookup(
......@@ -468,9 +470,6 @@ class V8_EXPORT_PRIVATE JSFunctionRef : public JSObjectRef {
bool has_instance_prototype(CompilationDependencies* dependencies) const;
ObjectRef instance_prototype(CompilationDependencies* dependencies) const;
MapRef initial_map(CompilationDependencies* dependencies) const;
ContextRef context(CompilationDependencies* dependencies) const;
NativeContextRef native_context(CompilationDependencies* dependencies) const;
SharedFunctionInfoRef shared(CompilationDependencies* dependencies) const;
int InitialMapInstanceSizeWithMinSlack(
CompilationDependencies* dependencies) const;
FeedbackVectorRef feedback_vector(
......@@ -479,9 +478,6 @@ class V8_EXPORT_PRIVATE JSFunctionRef : public JSObjectRef {
CompilationDependencies* dependencies) const;
CodeRef code() const;
private:
void RecordDependencyIfNeeded(CompilationDependencies* dependencies) const;
};
class RegExpBoilerplateDescriptionRef : public HeapObjectRef {
......
......@@ -2154,7 +2154,7 @@ TNode<Object> PromiseBuiltinReducerAssembler::ReducePromiseConstructor(
DCHECK_EQ(target, NewTargetInput());
SharedFunctionInfoRef promise_shared =
native_context.promise_function().shared(dependencies());
native_context.promise_function().shared();
PromiseCtorFrameStateParams frame_state_params{jsgraph(), promise_shared,
node_ptr(), context,
......@@ -2733,7 +2733,7 @@ Reduction JSCallReducer::ReduceFunctionPrototypeCall(Node* node) {
HeapObjectMatcher m(target);
if (m.HasResolvedValue() && m.Ref(broker()).IsJSFunction()) {
JSFunctionRef function = m.Ref(broker()).AsJSFunction();
context = jsgraph()->Constant(function.context(dependencies()));
context = jsgraph()->Constant(function.context());
} else {
context = effect = graph()->NewNode(
simplified()->LoadField(AccessBuilder::ForJSFunctionContext()), target,
......@@ -4277,8 +4277,8 @@ bool JSCallReducer::IsBuiltinOrApiFunction(JSFunctionRef function) const {
// TODO(neis): Add a way to check if function template info isn't serialized
// and add a warning in such cases. Currently we can't tell if function
// template info doesn't exist or wasn't serialized.
return function.shared(dependencies()).HasBuiltinId() ||
function.shared(dependencies()).function_template_info().has_value();
return function.shared().HasBuiltinId() ||
function.shared().function_template_info().has_value();
}
Reduction JSCallReducer::ReduceJSCall(Node* node) {
......@@ -4299,11 +4299,11 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
JSFunctionRef function = target_ref.AsJSFunction();
// Don't inline cross native context.
if (!function.native_context(dependencies()).equals(native_context())) {
if (!function.native_context().equals(native_context())) {
return NoChange();
}
return ReduceJSCall(node, function.shared(dependencies()));
return ReduceJSCall(node, function.shared());
} else if (target_ref.IsJSBoundFunction()) {
JSBoundFunctionRef function = target_ref.AsJSBoundFunction();
ObjectRef bound_this = function.bound_this();
......@@ -4992,11 +4992,11 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
// If this state changes during background compilation, the compilation
// job will be aborted from the main thread (see
// Debug::PrepareFunctionForDebugExecution()).
SharedFunctionInfoRef sfi = function.shared(dependencies());
SharedFunctionInfoRef sfi = function.shared();
if (sfi.HasBreakInfo()) return NoChange();
// Don't inline cross native context.
if (!function.native_context(dependencies()).equals(native_context())) {
if (!function.native_context().equals(native_context())) {
return NoChange();
}
......@@ -5042,8 +5042,7 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
case Builtin::kPromiseConstructor:
return ReducePromiseConstructor(node);
case Builtin::kTypedArrayConstructor:
return ReduceTypedArrayConstructor(node,
function.shared(dependencies()));
return ReduceTypedArrayConstructor(node, function.shared());
default:
break;
}
......@@ -8007,7 +8006,7 @@ Reduction JSCallReducer::ReduceNumberConstructor(Node* node) {
// Create the artificial frame state in the middle of the Number constructor.
SharedFunctionInfoRef shared_info =
native_context().number_function().shared(dependencies());
native_context().number_function().shared();
Node* stack_parameters[] = {receiver};
int stack_parameter_count = arraysize(stack_parameters);
Node* continuation_frame_state =
......
......@@ -399,7 +399,7 @@ Reduction JSCreateLowering::ReduceJSCreateGeneratorObject(Node* node) {
initial_map.instance_type() == JS_ASYNC_GENERATOR_OBJECT_TYPE);
// Allocate a register file.
SharedFunctionInfoRef shared = js_function.shared(dependencies());
SharedFunctionInfoRef shared = js_function.shared();
DCHECK(shared.HasBytecodeArray());
int parameter_count_no_receiver = shared.internal_formal_parameter_count();
int length = parameter_count_no_receiver +
......
......@@ -50,7 +50,7 @@ bool CanConsiderForInlining(JSHeapBroker* broker,
}
return CanConsiderForInlining(
broker, function.shared(broker->dependencies()),
broker, function.shared(),
function.feedback_vector(broker->dependencies()));
}
......@@ -68,7 +68,7 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
out.functions[0] = m.Ref(broker()).AsJSFunction();
JSFunctionRef function = out.functions[0].value();
if (CanConsiderForInlining(broker(), function)) {
out.bytecode[0] = function.shared(dependencies()).GetBytecodeArray();
out.bytecode[0] = function.shared().GetBytecodeArray();
out.num_functions = 1;
return out;
}
......@@ -89,7 +89,7 @@ JSInliningHeuristic::Candidate JSInliningHeuristic::CollectFunctions(
out.functions[n] = m.Ref(broker()).AsJSFunction();
JSFunctionRef function = out.functions[n].value();
if (CanConsiderForInlining(broker(), function)) {
out.bytecode[n] = function.shared(dependencies()).GetBytecodeArray();
out.bytecode[n] = function.shared().GetBytecodeArray();
}
}
out.num_functions = value_input_count;
......@@ -166,10 +166,9 @@ Reduction JSInliningHeuristic::Reduce(Node* node) {
continue;
}
SharedFunctionInfoRef shared =
candidate.functions[i].has_value()
? candidate.functions[i].value().shared(dependencies())
: candidate.shared_info.value();
SharedFunctionInfoRef shared = candidate.functions[i].has_value()
? candidate.functions[i].value().shared()
: candidate.shared_info.value();
candidate.can_inline_function[i] = candidate.bytecode[i].has_value();
CHECK_IMPLIES(candidate.can_inline_function[i], shared.IsInlineable());
// Do not allow direct recursion i.e. f() -> f(). We still allow indirect
......@@ -782,10 +781,9 @@ void JSInliningHeuristic::PrintCandidates() {
<< candidate.node->id() << " with frequency " << candidate.frequency
<< ", " << candidate.num_functions << " target(s):" << std::endl;
for (int i = 0; i < candidate.num_functions; ++i) {
SharedFunctionInfoRef shared =
candidate.functions[i].has_value()
? candidate.functions[i]->shared(dependencies())
: candidate.shared_info.value();
SharedFunctionInfoRef shared = candidate.functions[i].has_value()
? candidate.functions[i]->shared()
: candidate.shared_info.value();
os << " - target: " << shared;
if (candidate.bytecode[i].has_value()) {
os << ", bytecode size: " << candidate.bytecode[i]->length();
......
......@@ -317,12 +317,11 @@ base::Optional<SharedFunctionInfoRef> JSInliner::DetermineCallTarget(
// TODO(turbofan): We might want to revisit this restriction later when we
// have a need for this, and we know how to model different native contexts
// in the same graph in a compositional way.
if (!function.native_context(broker()->dependencies())
.equals(broker()->target_native_context())) {
if (!function.native_context().equals(broker()->target_native_context())) {
return base::nullopt;
}
return function.shared(broker()->dependencies());
return function.shared();
}
// This reducer can also handle calls where the target is statically known to
......@@ -359,8 +358,7 @@ FeedbackCellRef JSInliner::DetermineCallContext(Node* node,
CHECK(function.has_feedback_vector(broker()->dependencies()));
// The inlinee specializes to the context from the JSFunction object.
*context_out =
jsgraph()->Constant(function.context(broker()->dependencies()));
*context_out = jsgraph()->Constant(function.context());
return function.raw_feedback_cell(broker()->dependencies());
}
......
......@@ -1624,8 +1624,7 @@ Reduction JSTypedLowering::ReduceJSConstruct(Node* node) {
if (!function.map().is_constructor()) return NoChange();
// Patch {node} to an indirect call via the {function}s construct stub.
bool use_builtin_construct_stub =
function.shared(dependencies()).construct_as_builtin();
bool use_builtin_construct_stub = function.shared().construct_as_builtin();
CodeRef code = MakeRef(
broker(), use_builtin_construct_stub
? BUILTIN_CODE(isolate(), JSBuiltinsConstructStub)
......@@ -1701,7 +1700,7 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
if (target_type.IsHeapConstant() &&
target_type.AsHeapConstant()->Ref().IsJSFunction()) {
function = target_type.AsHeapConstant()->Ref().AsJSFunction();
shared = function->shared(dependencies());
shared = function->shared();
} else if (target->opcode() == IrOpcode::kJSCreateClosure) {
CreateClosureParameters const& ccp =
JSCreateClosureNode{target}.Parameters();
......@@ -1729,13 +1728,12 @@ Reduction JSTypedLowering::ReduceJSCall(Node* node) {
// require data from a foreign native context.
if (is_sloppy(shared->language_mode()) && !shared->native() &&
!receiver_type.Is(Type::Receiver())) {
if (!function.has_value() ||
!function->native_context(dependencies())
.equals(broker()->target_native_context())) {
if (!function.has_value() || !function->native_context().equals(
broker()->target_native_context())) {
return NoChange();
}
Node* global_proxy = jsgraph()->Constant(
function->native_context(dependencies()).global_proxy_object());
Node* global_proxy =
jsgraph()->Constant(function->native_context().global_proxy_object());
receiver = effect =
graph()->NewNode(simplified()->ConvertReceiver(convert_mode),
receiver, global_proxy, effect, control);
......
......@@ -1330,7 +1330,7 @@ struct GraphBuilderPhase {
JSFunctionRef closure = MakeRef(data->broker(), data->info()->closure());
CallFrequency frequency(1.0f);
BuildGraphFromBytecode(
data->broker(), temp_zone, closure.shared(data->dependencies()),
data->broker(), temp_zone, closure.shared(),
closure.raw_feedback_cell(data->dependencies()),
data->info()->osr_offset(), data->jsgraph(), frequency,
data->source_positions(), SourcePosition::kNotInlined,
......
......@@ -1515,10 +1515,10 @@ Type Typer::Visitor::JSCallTyper(Type fun, Typer* t) {
return Type::NonInternal();
}
JSFunctionRef function = fun.AsHeapConstant()->Ref().AsJSFunction();
if (!function.shared(t->broker()->dependencies()).HasBuiltinId()) {
if (!function.shared().HasBuiltinId()) {
return Type::NonInternal();
}
switch (function.shared(t->broker()->dependencies()).builtin_id()) {
switch (function.shared().builtin_id()) {
case Builtin::kMathRandom:
return Type::PlainNumber();
case Builtin::kMathFloor:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment