Fix error in compound assignment to keyed load by making platform-independent full...
authorwhesse@chromium.org <whesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 18 Feb 2010 12:59:41 +0000 (12:59 +0000)
committerwhesse@chromium.org <whesse@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Thu, 18 Feb 2010 12:59:41 +0000 (12:59 +0000)
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
src/full-codegen.cc
src/ia32/full-codegen-ia32.cc
src/x64/full-codegen-x64.cc
test/mjsunit/compiler/assignment.js [new file with mode: 0644]

index 55aac6d17091aaf17a04a2324aad80703b83c4fa..4896373802752eed1dec97d4f13170eddf25635e 100644 (file)
@@ -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();
index d7e250448f01c3d01db067b8766382896391f55e..63714392cd8768af8e9956d8298e9757133143cf 100644 (file)
@@ -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.
index df6ae8a7b6088b42c503750b3361e0b5efb3489b..2394bed62454d6ed4fdb756bae644b45c8f43c86 100644 (file)
@@ -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();
index b14cb854615679575e5aea83c761bc8d3a1f8e07..30db660de0e6c7cbffb9a6b1d802e9ec06559e19 100644 (file)
@@ -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 (file)
index 0000000..ee2d323
--- /dev/null
@@ -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();