Skip to content
This repository has been archived by the owner on Mar 25, 2018. It is now read-only.

Commit

Permalink
Revert of [fullcodegen] Refactor code that calls store ICs. (patchset #1
Browse files Browse the repository at this point in the history 
 id:20001 of https://codereview.chromium.org/2363513003/ )

Reason for revert:
race suspect:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20TSAN/builds/11893

Original issue's description:
> [fullcodegen] Refactor code that calls store ICs.
>
> Make FCG::CallStoreIC() load slot and name and make FCG::CallKeyedStoreIC() load
> slot according to store IC calling convention (StoreDescriptor).
>
> BUG=v8:5407
>
> Committed: https://crrev.com/12918397b4af7b2bede8b29e1e9b1940d5d5ad3b
> Cr-Commit-Position: refs/heads/master@{#39679}

TBR=mvstanton@chromium.org,ishell@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5407

Review-Url: https://codereview.chromium.org/2363123002
Cr-Commit-Position: refs/heads/master@{#39682}
  • Loading branch information
@mi-ac
mi-ac authored and Commit bot committed Sep 23, 2016
1 parent 0702ea3 commit 51fa56d
Show file tree
Hide file tree
Showing 11 changed files with 317 additions and 127 deletions.
45 changes: 34 additions & 11 deletions src/full-codegen/arm/full-codegen-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1168,9 +1168,12 @@ void FullCodeGenerator::EmitSetHomeObject(Expression* initializer, int offset,
FeedbackVectorSlot slot) {
DCHECK(NeedsHomeObject(initializer));
__ ldr(StoreDescriptor::ReceiverRegister(), MemOperand(sp));
__ mov(StoreDescriptor::NameRegister(),
Operand(isolate()->factory()->home_object_symbol()));
__ ldr(StoreDescriptor::ValueRegister(),
MemOperand(sp, offset * kPointerSize));
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
EmitLoadStoreICSlot(slot);
CallStoreIC();
}


Expand All @@ -1179,9 +1182,12 @@ void FullCodeGenerator::EmitSetHomeObjectAccumulator(Expression* initializer,
FeedbackVectorSlot slot) {
DCHECK(NeedsHomeObject(initializer));
__ Move(StoreDescriptor::ReceiverRegister(), r0);
__ mov(StoreDescriptor::NameRegister(),
Operand(isolate()->factory()->home_object_symbol()));
__ ldr(StoreDescriptor::ValueRegister(),
MemOperand(sp, offset * kPointerSize));
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
EmitLoadStoreICSlot(slot);
CallStoreIC();
}


Expand Down Expand Up @@ -1416,8 +1422,10 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
if (property->emit_store()) {
VisitForAccumulatorValue(value);
DCHECK(StoreDescriptor::ValueRegister().is(r0));
__ mov(StoreDescriptor::NameRegister(), Operand(key->value()));
__ ldr(StoreDescriptor::ReceiverRegister(), MemOperand(sp));
CallStoreIC(property->GetSlot(0), key->value());
EmitLoadStoreICSlot(property->GetSlot(0));
CallStoreIC();
PrepareForBailoutForId(key->id(), BailoutState::NO_REGISTERS);

if (NeedsHomeObject(value)) {
Expand Down Expand Up @@ -1615,7 +1623,8 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {

__ mov(StoreDescriptor::NameRegister(), Operand(Smi::FromInt(array_index)));
__ ldr(StoreDescriptor::ReceiverRegister(), MemOperand(sp, 0));
CallKeyedStoreIC(expr->LiteralFeedbackSlot());
EmitLoadStoreICSlot(expr->LiteralFeedbackSlot());
CallKeyedStoreIC();

PrepareForBailoutForId(expr->GetIdForElement(array_index),
BailoutState::NO_REGISTERS);
Expand Down Expand Up @@ -2047,7 +2056,10 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
VisitForAccumulatorValue(prop->obj());
__ Move(StoreDescriptor::ReceiverRegister(), r0);
PopOperand(StoreDescriptor::ValueRegister()); // Restore value.
CallStoreIC(slot, prop->key()->AsLiteral()->value());
__ mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
EmitLoadStoreICSlot(slot);
CallStoreIC();
break;
}
case NAMED_SUPER_PROPERTY: {
Expand Down Expand Up @@ -2094,7 +2106,8 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
__ Move(StoreDescriptor::NameRegister(), r0);
PopOperands(StoreDescriptor::ValueRegister(),
StoreDescriptor::ReceiverRegister());
CallKeyedStoreIC(slot);
EmitLoadStoreICSlot(slot);
CallKeyedStoreIC();
break;
}
}
Expand All @@ -2119,8 +2132,10 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var, Token::Value op,
FeedbackVectorSlot slot) {
if (var->IsUnallocated()) {
// Global var, const, or let.
__ mov(StoreDescriptor::NameRegister(), Operand(var->name()));
__ LoadGlobalObject(StoreDescriptor::ReceiverRegister());
CallStoreIC(slot, var->name());
EmitLoadStoreICSlot(slot);
CallStoreIC();

} else if (IsLexicalVariableMode(var->mode()) && op != Token::INIT) {
DCHECK(!var->IsLookupSlot());
Expand Down Expand Up @@ -2188,8 +2203,11 @@ void FullCodeGenerator::EmitNamedPropertyAssignment(Assignment* expr) {
DCHECK(prop != NULL);
DCHECK(prop->key()->IsLiteral());

__ mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
PopOperand(StoreDescriptor::ReceiverRegister());
CallStoreIC(expr->AssignmentSlot(), prop->key()->AsLiteral()->value());
EmitLoadStoreICSlot(expr->AssignmentSlot());
CallStoreIC();

PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
context()->Plug(r0);
Expand Down Expand Up @@ -2231,7 +2249,8 @@ void FullCodeGenerator::EmitKeyedPropertyAssignment(Assignment* expr) {
StoreDescriptor::NameRegister());
DCHECK(StoreDescriptor::ValueRegister().is(r0));

CallKeyedStoreIC(expr->AssignmentSlot());
EmitLoadStoreICSlot(expr->AssignmentSlot());
CallKeyedStoreIC();

PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
context()->Plug(r0);
Expand Down Expand Up @@ -3285,8 +3304,11 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
break;
case NAMED_PROPERTY: {
__ mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
PopOperand(StoreDescriptor::ReceiverRegister());
CallStoreIC(expr->CountSlot(), prop->key()->AsLiteral()->value());
EmitLoadStoreICSlot(expr->CountSlot());
CallStoreIC();
PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
if (expr->is_postfix()) {
if (!context()->IsEffect()) {
Expand Down Expand Up @@ -3324,7 +3346,8 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
case KEYED_PROPERTY: {
PopOperands(StoreDescriptor::ReceiverRegister(),
StoreDescriptor::NameRegister());
CallKeyedStoreIC(expr->CountSlot());
EmitLoadStoreICSlot(expr->CountSlot());
CallKeyedStoreIC();
PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
if (expr->is_postfix()) {
if (!context()->IsEffect()) {
Expand Down
45 changes: 34 additions & 11 deletions src/full-codegen/arm64/full-codegen-arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1158,8 +1158,11 @@ void FullCodeGenerator::EmitSetHomeObject(Expression* initializer, int offset,
FeedbackVectorSlot slot) {
DCHECK(NeedsHomeObject(initializer));
__ Peek(StoreDescriptor::ReceiverRegister(), 0);
__ Mov(StoreDescriptor::NameRegister(),
Operand(isolate()->factory()->home_object_symbol()));
__ Peek(StoreDescriptor::ValueRegister(), offset * kPointerSize);
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
EmitLoadStoreICSlot(slot);
CallStoreIC();
}


Expand All @@ -1168,8 +1171,11 @@ void FullCodeGenerator::EmitSetHomeObjectAccumulator(Expression* initializer,
FeedbackVectorSlot slot) {
DCHECK(NeedsHomeObject(initializer));
__ Move(StoreDescriptor::ReceiverRegister(), x0);
__ Mov(StoreDescriptor::NameRegister(),
Operand(isolate()->factory()->home_object_symbol()));
__ Peek(StoreDescriptor::ValueRegister(), offset * kPointerSize);
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
EmitLoadStoreICSlot(slot);
CallStoreIC();
}


Expand Down Expand Up @@ -1403,8 +1409,10 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
if (property->emit_store()) {
VisitForAccumulatorValue(value);
DCHECK(StoreDescriptor::ValueRegister().is(x0));
__ Mov(StoreDescriptor::NameRegister(), Operand(key->value()));
__ Peek(StoreDescriptor::ReceiverRegister(), 0);
CallStoreIC(property->GetSlot(0), key->value());
EmitLoadStoreICSlot(property->GetSlot(0));
CallStoreIC();
PrepareForBailoutForId(key->id(), BailoutState::NO_REGISTERS);

if (NeedsHomeObject(value)) {
Expand Down Expand Up @@ -1598,7 +1606,8 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) {

__ Mov(StoreDescriptor::NameRegister(), Smi::FromInt(array_index));
__ Peek(StoreDescriptor::ReceiverRegister(), 0);
CallKeyedStoreIC(expr->LiteralFeedbackSlot());
EmitLoadStoreICSlot(expr->LiteralFeedbackSlot());
CallKeyedStoreIC();

PrepareForBailoutForId(expr->GetIdForElement(array_index),
BailoutState::NO_REGISTERS);
Expand Down Expand Up @@ -1940,7 +1949,10 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
// this copy.
__ Mov(StoreDescriptor::ReceiverRegister(), x0);
PopOperand(StoreDescriptor::ValueRegister()); // Restore value.
CallStoreIC(slot, prop->key()->AsLiteral()->value());
__ Mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
EmitLoadStoreICSlot(slot);
CallStoreIC();
break;
}
case NAMED_SUPER_PROPERTY: {
Expand Down Expand Up @@ -1987,7 +1999,8 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
__ Mov(StoreDescriptor::NameRegister(), x0);
PopOperands(StoreDescriptor::ReceiverRegister(),
StoreDescriptor::ValueRegister());
CallKeyedStoreIC(slot);
EmitLoadStoreICSlot(slot);
CallKeyedStoreIC();
break;
}
}
Expand All @@ -2013,8 +2026,10 @@ void FullCodeGenerator::EmitVariableAssignment(Variable* var, Token::Value op,
ASM_LOCATION("FullCodeGenerator::EmitVariableAssignment");
if (var->IsUnallocated()) {
// Global var, const, or let.
__ Mov(StoreDescriptor::NameRegister(), Operand(var->name()));
__ LoadGlobalObject(StoreDescriptor::ReceiverRegister());
CallStoreIC(slot, var->name());
EmitLoadStoreICSlot(slot);
CallStoreIC();

} else if (IsLexicalVariableMode(var->mode()) && op != Token::INIT) {
DCHECK(!var->IsLookupSlot());
Expand Down Expand Up @@ -2080,8 +2095,11 @@ void FullCodeGenerator::EmitNamedPropertyAssignment(Assignment* expr) {
DCHECK(prop != NULL);
DCHECK(prop->key()->IsLiteral());

__ Mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
PopOperand(StoreDescriptor::ReceiverRegister());
CallStoreIC(expr->AssignmentSlot(), prop->key()->AsLiteral()->value());
EmitLoadStoreICSlot(expr->AssignmentSlot());
CallStoreIC();

PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
context()->Plug(x0);
Expand Down Expand Up @@ -2126,7 +2144,8 @@ void FullCodeGenerator::EmitKeyedPropertyAssignment(Assignment* expr) {
StoreDescriptor::ReceiverRegister());
DCHECK(StoreDescriptor::ValueRegister().is(x0));

CallKeyedStoreIC(expr->AssignmentSlot());
EmitLoadStoreICSlot(expr->AssignmentSlot());
CallKeyedStoreIC();

PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
context()->Plug(x0);
Expand Down Expand Up @@ -3207,8 +3226,11 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
}
break;
case NAMED_PROPERTY: {
__ Mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
PopOperand(StoreDescriptor::ReceiverRegister());
CallStoreIC(expr->CountSlot(), prop->key()->AsLiteral()->value());
EmitLoadStoreICSlot(expr->CountSlot());
CallStoreIC();
PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
if (expr->is_postfix()) {
if (!context()->IsEffect()) {
Expand Down Expand Up @@ -3246,7 +3268,8 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
case KEYED_PROPERTY: {
PopOperand(StoreDescriptor::NameRegister());
PopOperand(StoreDescriptor::ReceiverRegister());
CallKeyedStoreIC(expr->CountSlot());
EmitLoadStoreICSlot(expr->CountSlot());
CallKeyedStoreIC();
PrepareForBailoutForId(expr->AssignmentId(), BailoutState::TOS_REGISTER);
if (expr->is_postfix()) {
if (!context()->IsEffect()) {
Expand Down
32 changes: 11 additions & 21 deletions src/full-codegen/full-codegen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,37 +235,31 @@ void FullCodeGenerator::CallLoadGlobalIC(TypeofMode typeof_mode,
CallIC(ic, id);
}

void FullCodeGenerator::CallStoreIC(FeedbackVectorSlot slot,
Handle<Object> name, TypeFeedbackId id) {
DCHECK(name->IsName());
__ Move(StoreDescriptor::NameRegister(), name);
void FullCodeGenerator::CallStoreIC(TypeFeedbackId id) {
Handle<Code> ic = CodeFactory::StoreIC(isolate(), language_mode()).code();

STATIC_ASSERT(!StoreDescriptor::kPassLastArgsOnStack ||
StoreDescriptor::kStackArgumentsCount == 2);
if (StoreDescriptor::kPassLastArgsOnStack) {
__ Push(StoreDescriptor::ValueRegister());
EmitPushSlot(slot);
} else {
EmitLoadSlot(StoreDescriptor::SlotRegister(), slot);
__ Push(StoreDescriptor::SlotRegister());
}

Handle<Code> ic = CodeFactory::StoreIC(isolate(), language_mode()).code();
CallIC(ic, id);
RestoreContext();
}

void FullCodeGenerator::CallKeyedStoreIC(FeedbackVectorSlot slot) {
void FullCodeGenerator::CallKeyedStoreIC() {
Handle<Code> ic =
CodeFactory::KeyedStoreIC(isolate(), language_mode()).code();

STATIC_ASSERT(!StoreDescriptor::kPassLastArgsOnStack ||
StoreDescriptor::kStackArgumentsCount == 2);
if (StoreDescriptor::kPassLastArgsOnStack) {
__ Push(StoreDescriptor::ValueRegister());
EmitPushSlot(slot);
} else {
EmitLoadSlot(StoreDescriptor::SlotRegister(), slot);
__ Push(StoreDescriptor::SlotRegister());
}

Handle<Code> ic =
CodeFactory::KeyedStoreIC(isolate(), language_mode()).code();
CallIC(ic);
RestoreContext();
}
Expand Down Expand Up @@ -494,6 +488,7 @@ void FullCodeGenerator::VisitVariableProxy(VariableProxy* expr) {
EmitVariableLoad(expr);
}


void FullCodeGenerator::VisitSloppyBlockFunctionStatement(
SloppyBlockFunctionStatement* declaration) {
Visit(declaration->statement());
Expand Down Expand Up @@ -1134,14 +1129,9 @@ void FullCodeGenerator::EmitPropertyKey(LiteralProperty* property,
PushOperand(result_register());
}

void FullCodeGenerator::EmitLoadSlot(Register destination,
FeedbackVectorSlot slot) {
void FullCodeGenerator::EmitLoadStoreICSlot(FeedbackVectorSlot slot) {
DCHECK(!slot.IsInvalid());
__ Move(destination, SmiFromSlot(slot));
}

void FullCodeGenerator::EmitPushSlot(FeedbackVectorSlot slot) {
__ Push(SmiFromSlot(slot));
__ Move(StoreDescriptor::SlotRegister(), SmiFromSlot(slot));
}

void FullCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
Expand Down
10 changes: 3 additions & 7 deletions src/full-codegen/full-codegen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -613,10 +613,7 @@ class FullCodeGenerator final : public AstVisitor<FullCodeGenerator> {
void EmitSetHomeObjectAccumulator(Expression* initializer, int offset,
FeedbackVectorSlot slot);

// Platform-specific code for loading a slot to a register.
void EmitLoadSlot(Register destination, FeedbackVectorSlot slot);
// Platform-specific code for pushing a slot to the stack.
void EmitPushSlot(FeedbackVectorSlot slot);
void EmitLoadStoreICSlot(FeedbackVectorSlot slot);

void CallIC(Handle<Code> code,
TypeFeedbackId id = TypeFeedbackId::None());
Expand All @@ -625,9 +622,8 @@ class FullCodeGenerator final : public AstVisitor<FullCodeGenerator> {
// Inside typeof reference errors are never thrown.
void CallLoadGlobalIC(TypeofMode typeof_mode,
TypeFeedbackId id = TypeFeedbackId::None());
void CallStoreIC(FeedbackVectorSlot slot, Handle<Object> name,
TypeFeedbackId id = TypeFeedbackId::None());
void CallKeyedStoreIC(FeedbackVectorSlot slot);
void CallStoreIC(TypeFeedbackId id = TypeFeedbackId::None());
void CallKeyedStoreIC();

void SetFunctionPosition(FunctionLiteral* fun);
void SetReturnPosition(FunctionLiteral* fun);
Expand Down
Loading

0 comments on commit 51fa56d

Please sign in to comment.