Reviewers: Toon Verwaest,

Message:
Hi Toon, here is the issue we discussed, PTAL, thx!
--Michael

Description:
Fix for issue 351261.

This relands the following fix: "HAllocate should never generate
allocation code if the requested size does not fit into page. Regression
test included. (bug 347543)" along with additional fixes to KeyedStoreIC.

BUG=351261
LOG=N
[email protected]

Please review this at https://codereview.chromium.org/200113002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+37, -18 lines):
  M src/a64/lithium-codegen-a64.cc
  M src/arm/lithium-codegen-arm.cc
  M src/ia32/lithium-codegen-ia32.cc
  M src/ic.cc
  M src/mips/lithium-codegen-mips.cc
  M src/x64/lithium-codegen-x64.cc
  A + test/mjsunit/regress/regress-351261.js


Index: src/a64/lithium-codegen-a64.cc
diff --git a/src/a64/lithium-codegen-a64.cc b/src/a64/lithium-codegen-a64.cc
index 4766fa55fd078d3ad2d902732094fec8dc38cc0e..a0c0c7a804eb016f0990f7bd8d37d32148920e54 100644
--- a/src/a64/lithium-codegen-a64.cc
+++ b/src/a64/lithium-codegen-a64.cc
@@ -1507,7 +1507,11 @@ void LCodeGen::DoAllocate(LAllocate* instr) {

   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    __ Allocate(size, result, temp1, temp2, deferred->entry(), flags);
+    if (size <= Page::kMaxRegularHeapObjectSize) {
+      __ Allocate(size, result, temp1, temp2, deferred->entry(), flags);
+    } else {
+      __ B(deferred->entry());
+    }
   } else {
     Register size = ToRegister32(instr->size());
     __ Sxtw(size.X(), size);
Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 49b7a334e8ba744f84313c44f68c2857cc3e9ce4..2a1525810ee8eb14801d27301f96dde9cbc5dd15 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -5220,7 +5220,11 @@ void LCodeGen::DoAllocate(LAllocate* instr) {

   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
+    if (size <= Page::kMaxRegularHeapObjectSize) {
+ __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
+    } else {
+      __ jmp(deferred->entry());
+    }
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size,
Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index f85ab3d601ea36adaf1c61b3c9e082e66025a150..36e876dc22c19841d27f87bdadb9adf8df6c2f14 100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -5845,7 +5845,11 @@ void LCodeGen::DoAllocate(LAllocate* instr) {

   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
+    if (size <= Page::kMaxRegularHeapObjectSize) {
+      __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
+    } else {
+      __ jmp(deferred->entry());
+    }
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
Index: src/ic.cc
diff --git a/src/ic.cc b/src/ic.cc
index 7f4d1cd27046644f266fa33f61a45d0c378eabee..f1e3c5539d64fbd9f74cc0de671edc62a2cb3db6 100644
--- a/src/ic.cc
+++ b/src/ic.cc
@@ -1599,7 +1599,10 @@ KeyedAccessStoreMode KeyedStoreIC::GetStoreMode(Handle<JSObject> receiver,
   key->ToSmi()->To(&smi_key);
   int index = smi_key->value();
   bool oob_access = IsOutOfBoundsAccess(receiver, index);
-  bool allow_growth = receiver->IsJSArray() && oob_access;
+ // Don't consider this a growing store if the store would send the receiver to
+  // dictionary mode.
+  bool allow_growth = receiver->IsJSArray() && oob_access &&
+      !receiver->WouldConvertToSlowElements(key);
   if (allow_growth) {
     // Handle growing array in stub if necessary.
     if (receiver->HasFastSmiElements()) {
@@ -1724,12 +1727,7 @@ MaybeObject* KeyedStoreIC::Store(Handle<Object> object, if (!(receiver->map()->DictionaryElementsInPrototypeChainOnly())) {
             KeyedAccessStoreMode store_mode =
                 GetStoreMode(receiver, key, value);
-            // Use the generic stub if the store would send the receiver to
-            // dictionary mode.
-            if (!IsGrowStoreMode(store_mode) ||
-                !receiver->WouldConvertToSlowElements(key)) {
-              stub = StoreElementStub(receiver, store_mode);
-            }
+            stub = StoreElementStub(receiver, store_mode);
           }
         }
       }
Index: src/mips/lithium-codegen-mips.cc
diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 292ce13f24cc9d4b39de3bbbac953f91cef33032..6c5da81fb7d6b9e3761cd59c06600c926fec5097 100644
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -5191,7 +5191,11 @@ void LCodeGen::DoAllocate(LAllocate* instr) {
   }
   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
+    if (size <= Page::kMaxRegularHeapObjectSize) {
+ __ Allocate(size, result, scratch, scratch2, deferred->entry(), flags);
+    } else {
+      __ jmp(deferred->entry());
+    }
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size,
Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index f350b07d5bfc64ea2acb0330bcb8569eefdfeaa8..756bd655d0572e4b3b8c163787ef68a2dad4460a 100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -5145,7 +5145,11 @@ void LCodeGen::DoAllocate(LAllocate* instr) {

   if (instr->size()->IsConstantOperand()) {
     int32_t size = ToInteger32(LConstantOperand::cast(instr->size()));
-    __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
+    if (size <= Page::kMaxRegularHeapObjectSize) {
+      __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
+    } else {
+      __ jmp(deferred->entry());
+    }
   } else {
     Register size = ToRegister(instr->size());
     __ Allocate(size, result, temp, no_reg, deferred->entry(), flags);
Index: test/mjsunit/regress/regress-351261.js
diff --git a/test/mjsunit/regress/regress-346587.js b/test/mjsunit/regress/regress-351261.js
similarity index 57%
copy from test/mjsunit/regress/regress-346587.js
copy to test/mjsunit/regress/regress-351261.js
index 40e3ac116cdfd9984a7ac07bf09f357bc9e03506..48af5442fdc9b837a0259cceccdbfd3cf66f1a1d 100644
--- a/test/mjsunit/regress/regress-346587.js
+++ b/test/mjsunit/regress/regress-351261.js
@@ -2,17 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

-// Flags: --fold-constants --allow-natives-syntax
+// Flags: --allow-natives-syntax --fold-constants

-function bar(obj) {
-  assertTrue(obj.x === 'baz');
+function store(a) {
+  a[5000000] = 1;
 }

 function foo() {
-  bar({ x : 'baz' });
+  var __v_8 = new Object;
+  var __v_7 = new Array(4999990);
+  store(__v_8);
+  store(__v_7);
 }
-
-foo();
 foo();
 %OptimizeFunctionOnNextCall(foo);
 foo();


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to