[v8-dev] Do not inline array push for arrays with dictionary mode elements. (issue 880233002 by [email protected])

Wed, 28 Jan 2015 05:33:36 -0800 

Reviewers: mvstanton, danno,

Message:
Could you take a look, please?

The only thing that changed is the added !receiver_map->is_dictionary_map()
clause, the rest is just moving the checks to the CanInlineArrayResizeOperation 
method.

Description:
Do not inline array push for arrays with dictionary mode elements.

BUG=chromium:452878
LOG=n

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

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+26, -21 lines):
  M src/hydrogen.h
  M src/hydrogen.cc
  A + test/mjsunit/array-push12.js


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 54007c524a6cd376a357770eaeaaa28b6e04af8f..41845746c2fcacccb3bad2cfa839d6c3712271a1 100644 
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -8225,6 +8225,18 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinFunctionCall(Call* expr) { 
 }


+// static
+bool HOptimizedGraphBuilder::CanInlineArrayResizeOperation(
+    Handle<Map> receiver_map) {
+  return !receiver_map.is_null() &&
+         receiver_map->instance_type() == JS_ARRAY_TYPE &&
+         IsFastElementsKind(receiver_map->elements_kind()) &&
+         !receiver_map->is_dictionary_map() &&
+         !JSArray::IsReadOnlyLengthDescriptor(receiver_map) &&
+         !receiver_map->is_observed() && receiver_map->is_extensible();
+}
+
+
 bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall(
     Call* expr, Handle<JSFunction> function, Handle<Map> receiver_map,
     int args_count_no_receiver) {
@@ -8344,13 +8356,8 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall( 
       }
       break;
     case kArrayPop: {
-      if (receiver_map.is_null()) return false;
-      if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
+      if (!CanInlineArrayResizeOperation(receiver_map)) return false;
       ElementsKind elements_kind = receiver_map->elements_kind();
-      if (JSArray::IsReadOnlyLengthDescriptor(receiver_map)) return false;
-      if (!IsFastElementsKind(elements_kind)) return false;
-      if (receiver_map->is_observed()) return false;
-      if (!receiver_map->is_extensible()) return false;

       Drop(args_count_no_receiver);
       HValue* result;
@@ -8407,13 +8414,8 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall( 
       return true;
     }
     case kArrayPush: {
-      if (receiver_map.is_null()) return false;
-      if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
+      if (!CanInlineArrayResizeOperation(receiver_map)) return false;
       ElementsKind elements_kind = receiver_map->elements_kind();
-      if (!IsFastElementsKind(elements_kind)) return false;
-      if (receiver_map->is_observed()) return false;
-      if (JSArray::IsReadOnlyLengthDescriptor(receiver_map)) return false;
-      if (!receiver_map->is_extensible()) return false;
// If there may be elements accessors in the prototype chain, the fast 
       // inlined version can't be used.
@@ -8460,13 +8462,8 @@ bool HOptimizedGraphBuilder::TryInlineBuiltinMethodCall( 
       return true;
     }
     case kArrayShift: {
-      if (receiver_map.is_null()) return false;
-      if (receiver_map->instance_type() != JS_ARRAY_TYPE) return false;
+      if (!CanInlineArrayResizeOperation(receiver_map)) return false;
       ElementsKind kind = receiver_map->elements_kind();
-      if (JSArray::IsReadOnlyLengthDescriptor(receiver_map)) return false;
-      if (!IsFastElementsKind(kind)) return false;
-      if (receiver_map->is_observed()) return false;
-      if (!receiver_map->is_extensible()) return false;
// If there may be elements accessors in the prototype chain, the fast 
       // inlined version can't be used.
Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index 2a12edd8c4055bde38aabee71d15779fd494d995..51665a7a046cc72270fb26ec72b2cdcaffd64e12 100644 
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -2391,6 +2391,7 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor { 
                          int argc,
                          BailoutId ast_id,
                          ApiCallType call_type);
+  static bool CanInlineArrayResizeOperation(Handle<Map> receiver_map);

   // If --trace-inlining, print a line of the inlining trace.  Inlining
   // succeeded if the reason string is NULL and failed if there is a
Index: test/mjsunit/array-push12.js
diff --git a/test/mjsunit/compiler/regress-ntl-effect.js b/test/mjsunit/array-push12.js 
similarity index 63%
copy from test/mjsunit/compiler/regress-ntl-effect.js
copy to test/mjsunit/array-push12.js
index 708fe32828c9197dfa3d8c371ab01cbc1ad3317a..f4c15b484b41197c9007113ecf8f91fe909cc5d3 100644 
--- a/test/mjsunit/compiler/regress-ntl-effect.js
+++ b/test/mjsunit/array-push12.js
@@ -4,13 +4,20 @@

 // Flags: --allow-natives-syntax

+var a = [];
+for (var i = -20; i < 0; ++i) {
+  a[i] = 0;
+}
+
 function g() {
-  throw 0;
+    [].push.apply(a, arguments);
 }

 function f() {
   g();
-  while (1) {}
 }

-assertThrows(function () { f(); });
+g();
+g();
+%OptimizeFunctionOnNextCall(f);
+f();


--
--
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