Skip to content

Commit

Permalink
Reland "[Interpreter] Do not use IC slots for property load/stores in…
Browse files Browse the repository at this point in the history
… an IIFE and top-level code"

This is a reland of 690bda8

Original change's description:
> [Interpreter] Do not use IC slots for property load/stores in an IIFE and top-level code
> 
> An IIFE or top-level code is executed only once hence, there is no need to collect
> type feedback. We can save some memory by not using IC slots for property Loads/Stores
> within a IIFE/top-level code. This CL emits Runtime Get/Set property calls instead of LdaNamedProperty
> /StaNamedProperty for the property loads within a IIFE and top-level code.
> 
> Change-Id: I3e0ce26d05d82bb3648cb9262c4e112a2c4556c9
> Reviewed-on: https://chromium-review.googlesource.com/1146579
> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Commit-Queue: Chandan Reddy <chandanreddy@google.com>
> Cr-Commit-Position: refs/heads/master@{#54949}

Change-Id: I7b07ce86f7236d82191caaceafd31b86e5863ff5
Reviewed-on: https://chromium-review.googlesource.com/1167802
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Chandan Reddy <chandanreddy@google.com>
Cr-Commit-Position: refs/heads/master@{#55017}
  • Loading branch information
Creddy authored and Commit Bot committed Aug 9, 2018
1 parent 538bd6c commit 3b2b858
Show file tree
Hide file tree
Showing 18 changed files with 1,308 additions and 25 deletions.
9 changes: 8 additions & 1 deletion src/ast/ast.h
Expand Up @@ -2201,6 +2201,12 @@ class FunctionLiteral final : public Expression {
bool is_anonymous_expression() const {
return function_type() == kAnonymousExpression;
}

void mark_as_iife() { bit_field_ = IIFEBit::update(bit_field_, true); }
bool is_iife() const { return IIFEBit::decode(bit_field_); }
bool is_top_level() const {
return function_literal_id() == FunctionLiteral::kIdTypeTopLevel;
}
bool is_wrapped() const { return function_type() == kWrapped; }
LanguageMode language_mode() const;

Expand Down Expand Up @@ -2333,7 +2339,7 @@ class FunctionLiteral final : public Expression {
kHasDuplicateParameters) |
DontOptimizeReasonField::encode(BailoutReason::kNoReason) |
RequiresInstanceFieldsInitializer::encode(false) |
HasBracesField::encode(has_braces);
HasBracesField::encode(has_braces) | IIFEBit::encode(false);
if (eager_compile_hint == kShouldEagerCompile) SetShouldEagerCompile();
DCHECK_EQ(body == nullptr, expected_property_count < 0);
}
Expand All @@ -2348,6 +2354,7 @@ class FunctionLiteral final : public Expression {
: public BitField<bool, DontOptimizeReasonField::kNext, 1> {};
class HasBracesField
: public BitField<bool, RequiresInstanceFieldsInitializer::kNext, 1> {};
class IIFEBit : public BitField<bool, HasBracesField::kNext, 1> {};

int expected_property_count_;
int parameter_count_;
Expand Down
1 change: 1 addition & 0 deletions src/debug/debug-evaluate.cc
Expand Up @@ -277,6 +277,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
V(IsTypedArray) \
/* Loads */ \
V(LoadLookupSlotForCall) \
V(GetProperty) \
/* Arrays */ \
V(ArraySpeciesConstructor) \
V(EstimateNumberOfElements) \
Expand Down
5 changes: 5 additions & 0 deletions src/flag-definitions.h
Expand Up @@ -315,6 +315,11 @@ DEFINE_BOOL(optimize_for_size, false,
"Enables optimizations which favor memory size over execution "
"speed")

// Flag for one shot optimiztions.
DEFINE_BOOL(enable_one_shot_optimization, true,
"Enable size optimizations for the code that will "
"only be executed once")

DEFINE_VALUE_IMPLICATION(optimize_for_size, max_semi_space_size, 1)

// Flags for data representation optimizations
Expand Down
79 changes: 61 additions & 18 deletions src/interpreter/bytecode-generator.cc
Expand Up @@ -1815,6 +1815,14 @@ void BytecodeGenerator::AddToEagerLiteralsIfEager(FunctionLiteral* literal) {
}
}

bool BytecodeGenerator::ShouldOptimizeAsOneShot() const {
if (!FLAG_enable_one_shot_optimization) return false;

if (loop_depth_ > 0) return false;

return info()->literal()->is_top_level() || info()->literal()->is_iife();
}

void BytecodeGenerator::BuildClassLiteral(ClassLiteral* expr) {
size_t class_boilerplate_entry =
builder()->AllocateDeferredConstantPoolEntry();
Expand Down Expand Up @@ -2776,6 +2784,54 @@ void BytecodeGenerator::BuildVariableAssignment(
}
}

void BytecodeGenerator::BuildLoadNamedProperty(Property* property,
Register object,
const AstRawString* name) {
if (ShouldOptimizeAsOneShot()) {
RegisterList args = register_allocator()->NewRegisterList(2);
size_t name_index = builder()->GetConstantPoolEntry(name);
builder()
->MoveRegister(object, args[0])
.LoadConstantPoolEntry(name_index)
.StoreAccumulatorInRegister(args[1])
.CallRuntime(Runtime::kInlineGetProperty, args);
} else {
FeedbackSlot slot = GetCachedLoadICSlot(property->obj(), name);
builder()->LoadNamedProperty(object, name, feedback_index(slot));
}
}

void BytecodeGenerator::BuildStoreNamedProperty(Property* property,
Register object,
const AstRawString* name) {
Register value;
if (!execution_result()->IsEffect()) {
value = register_allocator()->NewRegister();
builder()->StoreAccumulatorInRegister(value);
}

if (ShouldOptimizeAsOneShot()) {
RegisterList args = register_allocator()->NewRegisterList(4);
size_t name_index = builder()->GetConstantPoolEntry(name);
builder()
->MoveRegister(object, args[0])
.StoreAccumulatorInRegister(args[2])
.LoadConstantPoolEntry(name_index)
.StoreAccumulatorInRegister(args[1])
.LoadLiteral(Smi::FromEnum(language_mode()))
.StoreAccumulatorInRegister(args[3])
.CallRuntime(Runtime::kSetProperty, args);
} else {
FeedbackSlot slot = GetCachedStoreICSlot(property->obj(), name);
builder()->StoreNamedProperty(object, name, feedback_index(slot),
language_mode());
}

if (!execution_result()->IsEffect()) {
builder()->LoadAccumulatorWithRegister(value);
}
}

void BytecodeGenerator::VisitAssignment(Assignment* expr) {
DCHECK(expr->target()->IsValidReferenceExpression() ||
(expr->op() == Token::INIT && expr->target()->IsVariableProxy() &&
Expand Down Expand Up @@ -2837,8 +2893,7 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
break;
}
case NAMED_PROPERTY: {
FeedbackSlot slot = GetCachedLoadICSlot(property->obj(), name);
builder()->LoadNamedProperty(object, name, feedback_index(slot));
BuildLoadNamedProperty(property, object, name);
break;
}
case KEYED_PROPERTY: {
Expand Down Expand Up @@ -2888,17 +2943,7 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
break;
}
case NAMED_PROPERTY: {
FeedbackSlot slot = GetCachedStoreICSlot(property->obj(), name);
Register value;
if (!execution_result()->IsEffect()) {
value = register_allocator()->NewRegister();
builder()->StoreAccumulatorInRegister(value);
}
builder()->StoreNamedProperty(object, name, feedback_index(slot),
language_mode());
if (!execution_result()->IsEffect()) {
builder()->LoadAccumulatorWithRegister(value);
}
BuildStoreNamedProperty(property, object, name);
break;
}
case KEYED_PROPERTY: {
Expand Down Expand Up @@ -3364,11 +3409,9 @@ void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* property) {
UNREACHABLE();
case NAMED_PROPERTY: {
builder()->SetExpressionPosition(property);
builder()->LoadNamedProperty(
obj, property->key()->AsLiteral()->AsRawPropertyName(),
feedback_index(GetCachedLoadICSlot(
property->obj(),
property->key()->AsLiteral()->AsRawPropertyName())));
const AstRawString* name =
property->key()->AsLiteral()->AsRawPropertyName();
BuildLoadNamedProperty(property, obj, name);
break;
}
case KEYED_PROPERTY: {
Expand Down
10 changes: 10 additions & 0 deletions src/interpreter/bytecode-generator.h
Expand Up @@ -120,6 +120,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
void VisitPropertyLoadForRegister(Register obj, Property* expr,
Register destination);

void BuildLoadNamedProperty(Property* property, Register object,
const AstRawString* name);
void BuildStoreNamedProperty(Property* property, Register object,
const AstRawString* name);

void BuildVariableLoad(Variable* variable, HoleCheckMode hole_check_mode,
TypeofMode typeof_mode = NOT_INSIDE_TYPEOF);
void BuildVariableLoadForAccumulatorValue(
Expand Down Expand Up @@ -277,6 +282,11 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {

void AddToEagerLiteralsIfEager(FunctionLiteral* literal);

// Checks if the visited expression is one shot, i.e executed only once. Any
// expression either in a top level code or an IIFE that is not within a loop
// is eligible for one shot optimizations.
inline bool ShouldOptimizeAsOneShot() const;

static constexpr ToBooleanMode ToBooleanModeFromTypeHint(TypeHint type_hint) {
return type_hint == TypeHint::kBoolean ? ToBooleanMode::kAlreadyBoolean
: ToBooleanMode::kConvertToBoolean;
Expand Down
6 changes: 6 additions & 0 deletions src/interpreter/interpreter-intrinsics-generator.cc
Expand Up @@ -224,6 +224,12 @@ Node* IntrinsicsGenerator::HasProperty(
args, context, Builtins::CallableFor(isolate(), Builtins::kHasProperty));
}

Node* IntrinsicsGenerator::GetProperty(
const InterpreterAssembler::RegListNodePair& args, Node* context) {
return IntrinsicAsStubCall(
args, context, Builtins::CallableFor(isolate(), Builtins::kGetProperty));
}

Node* IntrinsicsGenerator::RejectPromise(
const InterpreterAssembler::RegListNodePair& args, Node* context) {
return IntrinsicAsStubCall(
Expand Down
1 change: 1 addition & 0 deletions src/interpreter/interpreter-intrinsics.h
Expand Up @@ -26,6 +26,7 @@ namespace interpreter {
V(CreateIterResultObject, create_iter_result_object, 2) \
V(CreateAsyncFromSyncIterator, create_async_from_sync_iterator, 1) \
V(HasProperty, has_property, 2) \
V(GetProperty, get_property, 2) \
V(IsArray, is_array, 1) \
V(IsJSProxy, is_js_proxy, 1) \
V(IsJSReceiver, is_js_receiver, 1) \
Expand Down
1 change: 1 addition & 0 deletions src/parsing/parser-base.h
Expand Up @@ -3378,6 +3378,7 @@ ParserBase<Impl>::ParseLeftHandSideExpression(bool* ok) {
// function literal eagerly, we can also compile it eagerly.
if (result->IsFunctionLiteral()) {
result->AsFunctionLiteral()->SetShouldEagerCompile();
result->AsFunctionLiteral()->mark_as_iife();
}
}
Scanner::Location spread_pos;
Expand Down
1 change: 1 addition & 0 deletions src/parsing/preparser.h
Expand Up @@ -341,6 +341,7 @@ class PreParserExpression {

// More dummy implementations of things PreParser doesn't need to track:
void SetShouldEagerCompile() {}
void mark_as_iife() {}

int position() const { return kNoSourcePosition; }
void set_function_token_position(int position) {}
Expand Down
15 changes: 15 additions & 0 deletions test/cctest/interpreter/bytecode-expectations-printer.cc
Expand Up @@ -21,6 +21,7 @@
#include "src/objects/module-inl.h"
#include "src/runtime/runtime.h"
#include "src/source-position-table.h"
#include "test/cctest/cctest.h"

namespace v8 {
namespace internal {
Expand Down Expand Up @@ -116,6 +117,17 @@ BytecodeExpectationsPrinter::GetBytecodeArrayForScript(
return i::handle(js_function->shared()->GetBytecodeArray(), i_isolate());
}

i::Handle<i::BytecodeArray>
BytecodeExpectationsPrinter::GetBytecodeArrayOfCallee(
const char* source_code) const {
i::Handle<i::Object> i_object =
v8::Utils::OpenHandle(*CompileRun(source_code));
i::Handle<i::JSFunction> js_function =
i::Handle<i::JSFunction>::cast(i_object);
CHECK(js_function->shared()->HasBytecodeArray());
return i::handle(js_function->shared()->GetBytecodeArray(), i_isolate());
}

void BytecodeExpectationsPrinter::PrintEscapedString(
std::ostream& stream, const std::string& string) const {
for (char c : string) {
Expand Down Expand Up @@ -372,11 +384,14 @@ void BytecodeExpectationsPrinter::PrintExpectation(
wrap_ ? WrapCodeInFunction(test_function_name_.c_str(), snippet)
: snippet;

i::FLAG_enable_one_shot_optimization = oneshot_opt_;
i::Handle<i::BytecodeArray> bytecode_array;
if (module_) {
CHECK(top_level_ && !wrap_);
v8::Local<v8::Module> module = CompileModule(source_code.c_str());
bytecode_array = GetBytecodeArrayForModule(module);
} else if (print_callee_) {
bytecode_array = GetBytecodeArrayOfCallee(source_code.c_str());
} else {
v8::Local<v8::Script> script = CompileScript(source_code.c_str());
if (top_level_) {
Expand Down
12 changes: 12 additions & 0 deletions test/cctest/interpreter/bytecode-expectations-printer.h
Expand Up @@ -32,6 +32,8 @@ class BytecodeExpectationsPrinter final {
module_(false),
wrap_(true),
top_level_(false),
print_callee_(false),
oneshot_opt_(true),
test_function_name_(kDefaultTopFunctionName) {}

void PrintExpectation(std::ostream& stream, // NOLINT
Expand All @@ -46,6 +48,12 @@ class BytecodeExpectationsPrinter final {
void set_top_level(bool top_level) { top_level_ = top_level; }
bool top_level() const { return top_level_; }

void set_print_callee(bool print_callee) { print_callee_ = print_callee; }
bool print_callee() { return print_callee_; }

void set_oneshot_opt(bool oneshot_opt) { oneshot_opt_ = oneshot_opt; }
bool oneshot_opt() { return oneshot_opt_; }

void set_test_function_name(const std::string& test_function_name) {
test_function_name_ = test_function_name;
}
Expand Down Expand Up @@ -94,6 +102,8 @@ class BytecodeExpectationsPrinter final {
v8::Local<v8::Module> module) const;
i::Handle<v8::internal::BytecodeArray> GetBytecodeArrayForScript(
v8::Local<v8::Script> script) const;
i::Handle<i::BytecodeArray> GetBytecodeArrayOfCallee(
const char* source_code) const;

i::Isolate* i_isolate() const {
return reinterpret_cast<i::Isolate*>(isolate_);
Expand All @@ -103,6 +113,8 @@ class BytecodeExpectationsPrinter final {
bool module_;
bool wrap_;
bool top_level_;
bool print_callee_;
bool oneshot_opt_;
std::string test_function_name_;

static const char* const kDefaultTopFunctionName;
Expand Down

0 comments on commit 3b2b858

Please sign in to comment.