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

Commit

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

Reason for revert:
Didn't help

Original issue's description:
> Revert of [fullcodegen] Refactor code that calls store ICs. (patchset #1 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
>
> Committed: https://crrev.com/51fa56d1b8e9e320f7442682415e9df50ab19591
> Cr-Commit-Position: refs/heads/master@{#39682}

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/2361393005
Cr-Commit-Position: refs/heads/master@{#39683}
  • Loading branch information
@mi-ac
mi-ac authored and Commit bot committed Sep 23, 2016
1 parent 51fa56d commit f1e418e
Show file tree
Hide file tree
Showing 11 changed files with 127 additions and 317 deletions.
45 changes: 11 additions & 34 deletions src/full-codegen/arm/full-codegen-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1168,12 +1168,9 @@ 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));
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
}


Expand All @@ -1182,12 +1179,9 @@ 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));
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
}


Expand Down Expand Up @@ -1422,10 +1416,8 @@ 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));
EmitLoadStoreICSlot(property->GetSlot(0));
CallStoreIC();
CallStoreIC(property->GetSlot(0), key->value());
PrepareForBailoutForId(key->id(), BailoutState::NO_REGISTERS);

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

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

PrepareForBailoutForId(expr->GetIdForElement(array_index),
BailoutState::NO_REGISTERS);
Expand Down Expand Up @@ -2056,10 +2047,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
VisitForAccumulatorValue(prop->obj());
__ Move(StoreDescriptor::ReceiverRegister(), r0);
PopOperand(StoreDescriptor::ValueRegister()); // Restore value.
__ mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, prop->key()->AsLiteral()->value());
break;
}
case NAMED_SUPER_PROPERTY: {
Expand Down Expand Up @@ -2106,8 +2094,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
__ Move(StoreDescriptor::NameRegister(), r0);
PopOperands(StoreDescriptor::ValueRegister(),
StoreDescriptor::ReceiverRegister());
EmitLoadStoreICSlot(slot);
CallKeyedStoreIC();
CallKeyedStoreIC(slot);
break;
}
}
Expand All @@ -2132,10 +2119,8 @@ 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());
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, var->name());

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

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

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

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

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


Expand All @@ -1171,11 +1168,8 @@ 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);
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, isolate()->factory()->home_object_symbol());
}


Expand Down Expand Up @@ -1409,10 +1403,8 @@ 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);
EmitLoadStoreICSlot(property->GetSlot(0));
CallStoreIC();
CallStoreIC(property->GetSlot(0), key->value());
PrepareForBailoutForId(key->id(), BailoutState::NO_REGISTERS);

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

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

PrepareForBailoutForId(expr->GetIdForElement(array_index),
BailoutState::NO_REGISTERS);
Expand Down Expand Up @@ -1949,10 +1940,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
// this copy.
__ Mov(StoreDescriptor::ReceiverRegister(), x0);
PopOperand(StoreDescriptor::ValueRegister()); // Restore value.
__ Mov(StoreDescriptor::NameRegister(),
Operand(prop->key()->AsLiteral()->value()));
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, prop->key()->AsLiteral()->value());
break;
}
case NAMED_SUPER_PROPERTY: {
Expand Down Expand Up @@ -1999,8 +1987,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr,
__ Mov(StoreDescriptor::NameRegister(), x0);
PopOperands(StoreDescriptor::ReceiverRegister(),
StoreDescriptor::ValueRegister());
EmitLoadStoreICSlot(slot);
CallKeyedStoreIC();
CallKeyedStoreIC(slot);
break;
}
}
Expand All @@ -2026,10 +2013,8 @@ 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());
EmitLoadStoreICSlot(slot);
CallStoreIC();
CallStoreIC(slot, var->name());

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

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

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

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

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

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

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

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

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

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

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


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

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

void FullCodeGenerator::EmitPushSlot(FeedbackVectorSlot slot) {
__ Push(SmiFromSlot(slot));
}

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

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

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

0 comments on commit f1e418e

Please sign in to comment.