From 03dbdef1b30b99fb640d7a696a80fec94972f6f2 Mon Sep 17 00:00:00 2001 From: Adam Klein Date: Tue, 11 Jun 2024 09:32:46 -0700 Subject: [PATCH] [parser] Remove AST/Parser support for JS Runtime calls JS runtime calls are a vestige of the days when V8 had part of its runtime implemented in JavaScript. This cleanup CL removes parser/AST support JS runtime calls and removes the special-casing for those slots in native contexts. There are still two uses of CallJSRuntime in the interpreter, but they hardcode their context indices. This CL updates the unittests to hardcode the same indices for printing bytecode expectations. Change-Id: I05756bf6575bcb93b0e9a89407a7239fb0f5b30d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5590413 Commit-Queue: Adam Klein Auto-Submit: Adam Klein Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#94384} --- src/ast/ast.cc | 25 ------------- src/ast/ast.h | 35 ++++--------------- src/ast/prettyprinter.cc | 3 +- src/interpreter/bytecode-generator.cc | 16 +++------ src/objects/contexts.cc | 26 -------------- src/objects/contexts.h | 20 ++++------- src/parsing/parser.cc | 28 +++++---------- src/parsing/preparser.h | 6 ---- .../bytecode-expectations-printer.cc | 14 +++----- 9 files changed, 32 insertions(+), 141 deletions(-) diff --git a/src/ast/ast.cc b/src/ast/ast.cc index 586e100cb1..33e7155004 100644 --- a/src/ast/ast.cc +++ b/src/ast/ast.cc @@ -38,22 +38,6 @@ namespace internal { #ifdef DEBUG -static const char* NameForNativeContextIntrinsicIndex(uint32_t idx) { - switch (idx) { -#define NATIVE_CONTEXT_FIELDS_IDX(NAME, Type, name) \ - case Context::NAME: \ - return #name; - - NATIVE_CONTEXT_FIELDS(NATIVE_CONTEXT_FIELDS_IDX) -#undef NATIVE_CONTEXT_FIELDS_IDX - - default: - break; - } - - return "UnknownIntrinsicIndex"; -} - void AstNode::Print(Isolate* isolate) { AstPrinter::PrintOut(isolate, this); } @@ -1132,14 +1116,5 @@ Literal* AstNodeFactory::NewNumberLiteral(double number, int pos) { return zone_->New(number, pos); } -const char* CallRuntime::debug_name() { -#ifdef DEBUG - return is_jsruntime() ? NameForNativeContextIntrinsicIndex(context_index_) - : function_->name; -#else - return is_jsruntime() ? "(context function)" : function_->name; -#endif // DEBUG -} - } // namespace internal } // namespace v8 diff --git a/src/ast/ast.h b/src/ast/ast.h index ca5d3e0a37..080fab0a2b 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -1823,24 +1823,12 @@ class SuperCallForwardArgs final : public Expression { }; // The CallRuntime class does not represent any official JavaScript -// language construct. Instead it is used to call a C or JS function -// with a set of arguments. This is used from the builtins that are -// implemented in JavaScript. +// language construct. Instead it is used to call a runtime function +// with a set of arguments. class CallRuntime final : public Expression { public: const ZonePtrList* arguments() const { return &arguments_; } - bool is_jsruntime() const { return function_ == nullptr; } - - int context_index() const { - DCHECK(is_jsruntime()); - return context_index_; - } - const Runtime::Function* function() const { - DCHECK(!is_jsruntime()); - return function_; - } - - const char* debug_name(); + const Runtime::Function* function() const { return function_; } private: friend class AstNodeFactory; @@ -1850,15 +1838,10 @@ class CallRuntime final : public Expression { const ScopedPtrList& arguments, int pos) : Expression(pos, kCallRuntime), function_(function), - arguments_(arguments.ToConstVector(), zone) {} - CallRuntime(Zone* zone, int context_index, - const ScopedPtrList& arguments, int pos) - : Expression(pos, kCallRuntime), - context_index_(context_index), - function_(nullptr), - arguments_(arguments.ToConstVector(), zone) {} + arguments_(arguments.ToConstVector(), zone) { + DCHECK_NOT_NULL(function_); + } - int context_index_; const Runtime::Function* function_; ZonePtrList arguments_; }; @@ -3239,12 +3222,6 @@ class AstNodeFactory final { return zone_->New(zone_, function, arguments, pos); } - CallRuntime* NewCallRuntime(int context_index, - const ScopedPtrList& arguments, - int pos) { - return zone_->New(zone_, context_index, arguments, pos); - } - UnaryOperation* NewUnaryOperation(Token::Value op, Expression* expression, int pos) { diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc index 793d9cff9a..21a4ba93c4 100644 --- a/src/ast/prettyprinter.cc +++ b/src/ast/prettyprinter.cc @@ -1417,8 +1417,7 @@ void AstPrinter::VisitCallNew(CallNew* node) { void AstPrinter::VisitCallRuntime(CallRuntime* node) { base::EmbeddedVector buf; - SNPrintF(buf, "CALL RUNTIME %s%s", node->debug_name(), - node->is_jsruntime() ? " (JS function)" : ""); + SNPrintF(buf, "CALL RUNTIME %s", node->function()->name); IndentedScope indent(this, buf.begin(), node->position()); PrintArguments(node->arguments()); } diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 5323d4675b..9caf9108e6 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -6557,17 +6557,11 @@ void BytecodeGenerator::VisitSuperCallForwardArgs(SuperCallForwardArgs* expr) { } void BytecodeGenerator::VisitCallRuntime(CallRuntime* expr) { - if (expr->is_jsruntime()) { - RegisterList args = register_allocator()->NewGrowableRegisterList(); - VisitArguments(expr->arguments(), &args); - builder()->CallJSRuntime(expr->context_index(), args); - } else { - // Evaluate all arguments to the runtime call. - RegisterList args = register_allocator()->NewGrowableRegisterList(); - VisitArguments(expr->arguments(), &args); - Runtime::FunctionId function_id = expr->function()->function_id; - builder()->CallRuntime(function_id, args); - } + // Evaluate all arguments to the runtime call. + RegisterList args = register_allocator()->NewGrowableRegisterList(); + VisitArguments(expr->arguments(), &args); + Runtime::FunctionId function_id = expr->function()->function_id; + builder()->CallRuntime(function_id, args); } void BytecodeGenerator::VisitVoid(UnaryOperation* expr) { diff --git a/src/objects/contexts.cc b/src/objects/contexts.cc index 107c47a881..a13b118c59 100644 --- a/src/objects/contexts.cc +++ b/src/objects/contexts.cc @@ -561,32 +561,6 @@ Handle Context::ErrorMessageForWasmCodeGeneration() { "Wasm code generation disallowed by embedder"); } -#define COMPARE_NAME(index, type, name) \ - if (string->IsOneByteEqualTo(base::StaticCharVector(#name))) return index; - -int Context::IntrinsicIndexForName(DirectHandle string) { - NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(COMPARE_NAME); - return kNotFound; -} - -#undef COMPARE_NAME - -#define COMPARE_NAME(index, type, name) \ - { \ - const int name_length = static_cast(arraysize(#name)) - 1; \ - if ((length == name_length) && strncmp(string, #name, name_length) == 0) \ - return index; \ - } - -int Context::IntrinsicIndexForName(const unsigned char* unsigned_string, - int length) { - const char* string = reinterpret_cast(unsigned_string); - NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(COMPARE_NAME); - return kNotFound; -} - -#undef COMPARE_NAME - #ifdef VERIFY_HEAP namespace { // TODO(v8:12298): Fix js-context-specialization cctests to set up full diff --git a/src/objects/contexts.h b/src/objects/contexts.h index abe9b9f69d..7ced2dbc0d 100644 --- a/src/objects/contexts.h +++ b/src/objects/contexts.h @@ -41,15 +41,6 @@ enum ContextLookupFlags { // must always be allocated via Heap::AllocateContext() or // Factory::NewContext. -#define NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(V) \ - V(GENERATOR_NEXT_INTERNAL, JSFunction, generator_next_internal) \ - V(ASYNC_MODULE_EVALUATE_INTERNAL, JSFunction, \ - async_module_evaluate_internal) \ - V(REFLECT_APPLY_INDEX, JSFunction, reflect_apply) \ - V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct) \ - V(PROMISE_THEN_INDEX, JSFunction, promise_then) \ - V(FUNCTION_PROTOTYPE_APPLY_INDEX, JSFunction, function_prototype_apply) - #define NATIVE_CONTEXT_FIELDS(V) \ V(GLOBAL_PROXY_INDEX, JSGlobalProxy, global_proxy_object) \ /* TODO(ishell): Actually we store exactly EmbedderDataArray here but */ \ @@ -58,7 +49,13 @@ enum ContextLookupFlags { V(EMBEDDER_DATA_INDEX, HeapObject, embedder_data) \ V(CONTINUATION_PRESERVED_EMBEDDER_DATA_INDEX, HeapObject, \ continuation_preserved_embedder_data) \ - NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(V) \ + V(GENERATOR_NEXT_INTERNAL, JSFunction, generator_next_internal) \ + V(ASYNC_MODULE_EVALUATE_INTERNAL, JSFunction, \ + async_module_evaluate_internal) \ + V(REFLECT_APPLY_INDEX, JSFunction, reflect_apply) \ + V(REFLECT_CONSTRUCT_INDEX, JSFunction, reflect_construct) \ + V(PROMISE_THEN_INDEX, JSFunction, promise_then) \ + V(FUNCTION_PROTOTYPE_APPLY_INDEX, JSFunction, function_prototype_apply) \ /* TypedArray constructors - these must stay in order! */ \ V(UINT8_ARRAY_FUN_INDEX, JSFunction, uint8_array_fun) \ V(INT8_ARRAY_FUN_INDEX, JSFunction, int8_array_fun) \ @@ -623,9 +620,6 @@ class Context : public TorqueGeneratedContext { Handle ErrorMessageForCodeGenerationFromStrings(); Handle ErrorMessageForWasmCodeGeneration(); - static int IntrinsicIndexForName(DirectHandle name); - static int IntrinsicIndexForName(const unsigned char* name, int length); - #define NATIVE_CONTEXT_FIELD_ACCESSORS(index, type, name) \ inline void set_##name(Tagged value); \ inline bool is_##name(Tagged value) const; \ diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index b246b8f455..f67fe68bd5 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -409,30 +409,18 @@ Expression* Parser::NewV8Intrinsic(const AstRawString* name, return NewV8RuntimeFunctionForFuzzing(function, args, pos); } - if (function != nullptr) { - // Check for possible name clash. - DCHECK_EQ(Context::kNotFound, - Context::IntrinsicIndexForName(name->raw_data(), name->length())); - - // Check that the expected number of arguments are being passed. - if (function->nargs != -1 && function->nargs != args.length()) { - ReportMessage(MessageTemplate::kRuntimeWrongNumArgs); - return FailureExpression(); - } - - return factory()->NewCallRuntime(function, args, pos); - } - - int context_index = - Context::IntrinsicIndexForName(name->raw_data(), name->length()); - - // Check that the function is defined. - if (context_index == Context::kNotFound) { + if (function == nullptr) { ReportMessage(MessageTemplate::kNotDefined, name); return FailureExpression(); } - return factory()->NewCallRuntime(context_index, args, pos); + // Check that the expected number of arguments are being passed. + if (function->nargs != -1 && function->nargs != args.length()) { + ReportMessage(MessageTemplate::kRuntimeWrongNumArgs); + return FailureExpression(); + } + + return factory()->NewCallRuntime(function, args, pos); } // More permissive runtime-function creation on fuzzers. diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h index 5822bf4f06..4fcaa914c9 100644 --- a/src/parsing/preparser.h +++ b/src/parsing/preparser.h @@ -713,12 +713,6 @@ class PreParserFactory { return PreParserStatement::Iteration(); } - PreParserExpression NewCallRuntime( - Runtime::FunctionId id, ZoneChunkList* arguments, - int pos) { - return PreParserExpression::Default(); - } - PreParserExpression NewImportCallExpression(const PreParserExpression& args, int pos) { return PreParserExpression::Default(); diff --git a/test/unittests/interpreter/bytecode-expectations-printer.cc b/test/unittests/interpreter/bytecode-expectations-printer.cc index 8a0bffcbe5..81c5f40c5a 100644 --- a/test/unittests/interpreter/bytecode-expectations-printer.cc +++ b/test/unittests/interpreter/bytecode-expectations-printer.cc @@ -30,17 +30,13 @@ namespace interpreter { static const char* NameForNativeContextIntrinsicIndex(uint32_t idx) { switch (idx) { -#define COMPARE_NATIVE_CONTEXT_INTRINSIC_IDX(NAME, Type, name) \ - case Context::NAME: \ - return #name; - - NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(COMPARE_NATIVE_CONTEXT_INTRINSIC_IDX) - + case Context::REFLECT_APPLY_INDEX: + return "reflect_apply"; + case Context::REFLECT_CONSTRUCT_INDEX: + return "reflect_construct"; default: - break; + return "UnknownIntrinsicIndex"; } - - return "UnknownIntrinsicIndex"; } // static