From b9a250d2e6e20f6e4653ed58d48e45b03a1f3e46 Mon Sep 17 00:00:00 2001 From: "whesse@chromium.org" Date: Thu, 18 Feb 2010 12:59:41 +0000 Subject: [PATCH] Fix error in compound assignment to keyed load by making platform-independent full compiler code platform dependent, add test of compound assignments. Review URL: http://codereview.chromium.org/646009 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3897 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 86 +++++++++ src/full-codegen.cc | 86 --------- src/ia32/full-codegen-ia32.cc | 93 ++++++++++ src/x64/full-codegen-x64.cc | 86 +++++++++ test/mjsunit/compiler/assignment.js | 264 ++++++++++++++++++++++++++++ 5 files changed, 529 insertions(+), 86 deletions(-) create mode 100644 test/mjsunit/compiler/assignment.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 55aac6d170..4896373802 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -907,6 +907,92 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } +void FullCodeGenerator::VisitAssignment(Assignment* expr) { + Comment cmnt(masm_, "[ Assignment"); + ASSERT(expr->op() != Token::INIT_CONST); + // Left-hand side can only be a property, a global or a (parameter or local) + // slot. Variables with rewrite to .arguments are treated as KEYED_PROPERTY. + enum LhsKind { VARIABLE, NAMED_PROPERTY, KEYED_PROPERTY }; + LhsKind assign_type = VARIABLE; + Property* prop = expr->target()->AsProperty(); + if (prop != NULL) { + assign_type = + (prop->key()->IsPropertyName()) ? NAMED_PROPERTY : KEYED_PROPERTY; + } + + // Evaluate LHS expression. + switch (assign_type) { + case VARIABLE: + // Nothing to do here. + break; + case NAMED_PROPERTY: + if (expr->is_compound()) { + // We need the receiver both on the stack and in the accumulator. + VisitForValue(prop->obj(), kAccumulator); + __ push(result_register()); + } else { + VisitForValue(prop->obj(), kStack); + } + break; + case KEYED_PROPERTY: + VisitForValue(prop->obj(), kStack); + VisitForValue(prop->key(), kStack); + break; + } + + // If we have a compound assignment: Get value of LHS expression and + // store in on top of the stack. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kStack; + switch (assign_type) { + case VARIABLE: + EmitVariableLoad(expr->target()->AsVariableProxy()->var(), + Expression::kValue); + break; + case NAMED_PROPERTY: + EmitNamedPropertyLoad(prop); + __ push(result_register()); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyLoad(prop); + __ push(result_register()); + break; + } + location_ = saved_location; + } + + // Evaluate RHS expression. + Expression* rhs = expr->value(); + VisitForValue(rhs, kAccumulator); + + // If we have a compound assignment: Apply operator. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kAccumulator; + EmitBinaryOp(expr->binary_op(), Expression::kValue); + location_ = saved_location; + } + + // Record source position before possible IC call. + SetSourcePosition(expr->position()); + + // Store the value. + switch (assign_type) { + case VARIABLE: + EmitVariableAssignment(expr->target()->AsVariableProxy()->var(), + context_); + break; + case NAMED_PROPERTY: + EmitNamedPropertyAssignment(expr); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyAssignment(expr); + break; + } +} + + void FullCodeGenerator::EmitNamedPropertyLoad(Property* prop) { SetSourcePosition(prop->position()); Literal* key = prop->key()->AsLiteral(); diff --git a/src/full-codegen.cc b/src/full-codegen.cc index d7e250448f..63714392cd 100644 --- a/src/full-codegen.cc +++ b/src/full-codegen.cc @@ -1036,92 +1036,6 @@ void FullCodeGenerator::VisitLiteral(Literal* expr) { } -void FullCodeGenerator::VisitAssignment(Assignment* expr) { - Comment cmnt(masm_, "[ Assignment"); - ASSERT(expr->op() != Token::INIT_CONST); - // Left-hand side can only be a property, a global or a (parameter or local) - // slot. Variables with rewrite to .arguments are treated as KEYED_PROPERTY. - enum LhsKind { VARIABLE, NAMED_PROPERTY, KEYED_PROPERTY }; - LhsKind assign_type = VARIABLE; - Property* prop = expr->target()->AsProperty(); - if (prop != NULL) { - assign_type = - (prop->key()->IsPropertyName()) ? NAMED_PROPERTY : KEYED_PROPERTY; - } - - // Evaluate LHS expression. - switch (assign_type) { - case VARIABLE: - // Nothing to do here. - break; - case NAMED_PROPERTY: - if (expr->is_compound()) { - // We need the receiver both on the stack and in the accumulator. - VisitForValue(prop->obj(), kAccumulator); - __ push(result_register()); - } else { - VisitForValue(prop->obj(), kStack); - } - break; - case KEYED_PROPERTY: - VisitForValue(prop->obj(), kStack); - VisitForValue(prop->key(), kStack); - break; - } - - // If we have a compound assignment: Get value of LHS expression and - // store in on top of the stack. - if (expr->is_compound()) { - Location saved_location = location_; - location_ = kStack; - switch (assign_type) { - case VARIABLE: - EmitVariableLoad(expr->target()->AsVariableProxy()->var(), - Expression::kValue); - break; - case NAMED_PROPERTY: - EmitNamedPropertyLoad(prop); - __ push(result_register()); - break; - case KEYED_PROPERTY: - EmitKeyedPropertyLoad(prop); - __ push(result_register()); - break; - } - location_ = saved_location; - } - - // Evaluate RHS expression. - Expression* rhs = expr->value(); - VisitForValue(rhs, kAccumulator); - - // If we have a compound assignment: Apply operator. - if (expr->is_compound()) { - Location saved_location = location_; - location_ = kAccumulator; - EmitBinaryOp(expr->binary_op(), Expression::kValue); - location_ = saved_location; - } - - // Record source position before possible IC call. - SetSourcePosition(expr->position()); - - // Store the value. - switch (assign_type) { - case VARIABLE: - EmitVariableAssignment(expr->target()->AsVariableProxy()->var(), - context_); - break; - case NAMED_PROPERTY: - EmitNamedPropertyAssignment(expr); - break; - case KEYED_PROPERTY: - EmitKeyedPropertyAssignment(expr); - break; - } -} - - void FullCodeGenerator::VisitCatchExtensionObject(CatchExtensionObject* expr) { // Call runtime routine to allocate the catch extension object and // assign the exception value to the catch variable. diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index df6ae8a7b6..2394bed624 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -1013,6 +1013,99 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } +void FullCodeGenerator::VisitAssignment(Assignment* expr) { + Comment cmnt(masm_, "[ Assignment"); + ASSERT(expr->op() != Token::INIT_CONST); + // Left-hand side can only be a property, a global or a (parameter or local) + // slot. Variables with rewrite to .arguments are treated as KEYED_PROPERTY. + enum LhsKind { VARIABLE, NAMED_PROPERTY, KEYED_PROPERTY }; + LhsKind assign_type = VARIABLE; + Property* prop = expr->target()->AsProperty(); + if (prop != NULL) { + assign_type = + (prop->key()->IsPropertyName()) ? NAMED_PROPERTY : KEYED_PROPERTY; + } + + // Evaluate LHS expression. + switch (assign_type) { + case VARIABLE: + // Nothing to do here. + break; + case NAMED_PROPERTY: + if (expr->is_compound()) { + // We need the receiver both on the stack and in the accumulator. + VisitForValue(prop->obj(), kAccumulator); + __ push(result_register()); + } else { + VisitForValue(prop->obj(), kStack); + } + break; + case KEYED_PROPERTY: + if (expr->is_compound()) { + VisitForValue(prop->obj(), kStack); + VisitForValue(prop->key(), kAccumulator); + __ mov(edx, Operand(esp, 0)); + __ push(eax); + } else { + VisitForValue(prop->obj(), kStack); + VisitForValue(prop->key(), kStack); + } + break; + } + + // If we have a compound assignment: Get value of LHS expression and + // store in on top of the stack. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kStack; + switch (assign_type) { + case VARIABLE: + EmitVariableLoad(expr->target()->AsVariableProxy()->var(), + Expression::kValue); + break; + case NAMED_PROPERTY: + EmitNamedPropertyLoad(prop); + __ push(result_register()); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyLoad(prop); + __ push(result_register()); + break; + } + location_ = saved_location; + } + + // Evaluate RHS expression. + Expression* rhs = expr->value(); + VisitForValue(rhs, kAccumulator); + + // If we have a compound assignment: Apply operator. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kAccumulator; + EmitBinaryOp(expr->binary_op(), Expression::kValue); + location_ = saved_location; + } + + // Record source position before possible IC call. + SetSourcePosition(expr->position()); + + // Store the value. + switch (assign_type) { + case VARIABLE: + EmitVariableAssignment(expr->target()->AsVariableProxy()->var(), + context_); + break; + case NAMED_PROPERTY: + EmitNamedPropertyAssignment(expr); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyAssignment(expr); + break; + } +} + + void FullCodeGenerator::EmitNamedPropertyLoad(Property* prop) { SetSourcePosition(prop->position()); Literal* key = prop->key()->AsLiteral(); diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index b14cb85461..30db660de0 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -1015,6 +1015,92 @@ void FullCodeGenerator::VisitArrayLiteral(ArrayLiteral* expr) { } +void FullCodeGenerator::VisitAssignment(Assignment* expr) { + Comment cmnt(masm_, "[ Assignment"); + ASSERT(expr->op() != Token::INIT_CONST); + // Left-hand side can only be a property, a global or a (parameter or local) + // slot. Variables with rewrite to .arguments are treated as KEYED_PROPERTY. + enum LhsKind { VARIABLE, NAMED_PROPERTY, KEYED_PROPERTY }; + LhsKind assign_type = VARIABLE; + Property* prop = expr->target()->AsProperty(); + if (prop != NULL) { + assign_type = + (prop->key()->IsPropertyName()) ? NAMED_PROPERTY : KEYED_PROPERTY; + } + + // Evaluate LHS expression. + switch (assign_type) { + case VARIABLE: + // Nothing to do here. + break; + case NAMED_PROPERTY: + if (expr->is_compound()) { + // We need the receiver both on the stack and in the accumulator. + VisitForValue(prop->obj(), kAccumulator); + __ push(result_register()); + } else { + VisitForValue(prop->obj(), kStack); + } + break; + case KEYED_PROPERTY: + VisitForValue(prop->obj(), kStack); + VisitForValue(prop->key(), kStack); + break; + } + + // If we have a compound assignment: Get value of LHS expression and + // store in on top of the stack. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kStack; + switch (assign_type) { + case VARIABLE: + EmitVariableLoad(expr->target()->AsVariableProxy()->var(), + Expression::kValue); + break; + case NAMED_PROPERTY: + EmitNamedPropertyLoad(prop); + __ push(result_register()); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyLoad(prop); + __ push(result_register()); + break; + } + location_ = saved_location; + } + + // Evaluate RHS expression. + Expression* rhs = expr->value(); + VisitForValue(rhs, kAccumulator); + + // If we have a compound assignment: Apply operator. + if (expr->is_compound()) { + Location saved_location = location_; + location_ = kAccumulator; + EmitBinaryOp(expr->binary_op(), Expression::kValue); + location_ = saved_location; + } + + // Record source position before possible IC call. + SetSourcePosition(expr->position()); + + // Store the value. + switch (assign_type) { + case VARIABLE: + EmitVariableAssignment(expr->target()->AsVariableProxy()->var(), + context_); + break; + case NAMED_PROPERTY: + EmitNamedPropertyAssignment(expr); + break; + case KEYED_PROPERTY: + EmitKeyedPropertyAssignment(expr); + break; + } +} + + void FullCodeGenerator::EmitNamedPropertyLoad(Property* prop) { SetSourcePosition(prop->position()); Literal* key = prop->key()->AsLiteral(); diff --git a/test/mjsunit/compiler/assignment.js b/test/mjsunit/compiler/assignment.js new file mode 100644 index 0000000000..ee2d323781 --- /dev/null +++ b/test/mjsunit/compiler/assignment.js @@ -0,0 +1,264 @@ +// Copyright 2010 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Tests for compound assignments at the top level + +z = 2; +z += 4; + +assertEquals(z, 6); + +a = new Array(10); + +a[2] += 7; +a[2] = 15; +a[2] += 2; + +assertEquals(17, a[2]); + +b = new Object(); +b.foo = 5; +b.foo += 12; + +assertEquals(17, b.foo); + +// Test compound assignments in an anonymous function with local variables. +(function () { + var z = 2; + z += 4; + + assertEquals(z, 6); + + var a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + var b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); +})(); + +// Test compound assignments in an anonymous function with global variables. +(function () { + z = 2; + z += 4; + + assertEquals(z, 6); + + a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); +})(); + +// Test compound assignments in a named function with local variables. +function foo() { + var z = 3; + z += 4; + + assertEquals(z, 7); + + var a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + var b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); +} + +foo(); + +// Test compound assignments in a named function with global variables. +function bar() { + z = 2; + z += 5; + + assertEquals(z, 7); + + a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); +} + +bar(); + +// Entire series of tests repeated, in loops. +// ------------------------------------------- +// Tests for compound assignments in a loop at the top level + +for (i = 0; i < 5; ++i) { + z = 2; + z += 4; + + assertEquals(z, 6); + + a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); +} + +// Test compound assignments in an anonymous function with local variables. +(function () { + for (var i = 0; i < 5; ++i) { + var z = 2; + z += 4; + + assertEquals(z, 6); + + var a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + var b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); + } +})(); + +// Test compound assignments in an anonymous function with global variables. +(function () { + for (i = 0; i < 5; ++i) { + z = 2; + z += 4; + + assertEquals(z, 6); + + a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); + } +})(); + +// Test compound assignments in a named function with local variables. +function foo_loop() { + for (i = 0; i < 5; ++i) { + var z = 3; + z += 4; + + assertEquals(z, 7); + + var a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + var b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); + } +} + +foo_loop(); + +// Test compound assignments in a named function with global variables. +function bar_loop() { + for (i = 0; i < 5; ++i) { + z = 2; + z += 5; + + assertEquals(z, 7); + + a = new Array(10); + + a[2] += 7; + a[2] = 15; + a[2] += 2; + + assertEquals(17, a[2]); + + b = new Object(); + b.foo = 5; + b.foo += 12; + + assertEquals(17, b.foo); + } +} + +bar_loop();