[es2015] Optimize Reflect.has builtin.

Port the baseline version of Reflect.has to the CodeStubAssembler and
reuse the existing logic for HasProperty (i.e. the HasProperty builtin).
Also inline the Reflect.has builtin into TurboFan, by adding a check
on the target in front of a use of the JSHasProperty operator.
Technically this additional check is not necessary, because the
JSHasProperty operator already throws if the target is not a JSReceiver,
but the exception message is confusing then.

This improves the performance of the micro-benchmark from

  reflectHasPresent: 337 ms.
  reflectHasAbsent: 472 ms.

to

  reflectHasPresent: 121 ms.
  reflectHasAbsent: 216 ms.

which is a nice 2.8x improvement in the best case. It also improves the
chai test on the web-tooling-benchmark by around 1-2%, which is roughly
the expected win (since Reflect.has overall accounts for around 3-4%).

Bug: v8:5996, v8:6936, v8:6937
Change-Id: I856183229677a71c19936f06f2a4fc7a794a9a4a
Reviewed-on: https://chromium-review.googlesource.com/720959
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48608}

BUILD.gn

@@ -1016,6 +1016,7 @@ v8_source_set("v8_initializers") {
     "src/builtins/builtins-promise-gen.h",
     "src/builtins/builtins-proxy-gen.cc",
     "src/builtins/builtins-proxy-gen.h",
+    "src/builtins/builtins-reflect-gen.cc",
     "src/builtins/builtins-regexp-gen.cc",
     "src/builtins/builtins-regexp-gen.h",
     "src/builtins/builtins-sharedarraybuffer-gen.cc",

src/builtins/builtins-definitions.h

@@ -795,7 +795,7 @@ namespace internal {
   CPP(ReflectGet)                                                              \
   CPP(ReflectGetOwnPropertyDescriptor)                                         \
   CPP(ReflectGetPrototypeOf)                                                   \
-  CPP(ReflectHas)                                                              \
+  TFJ(ReflectHas, 2, kTarget, kKey)                                            \
   CPP(ReflectIsExtensible)                                                     \
   CPP(ReflectOwnKeys)                                                          \
   CPP(ReflectPreventExtensions)                                                \

src/builtins/builtins-promise-gen.cc

@@ -237,28 +237,6 @@ Node* PromiseBuiltinsAssembler::CreatePromiseGetCapabilitiesExecutorContext(
   return context;
 }
 
-Node* PromiseBuiltinsAssembler::ThrowIfNotJSReceiver(
-    Node* context, Node* value, MessageTemplate::Template msg_template,
-    const char* method_name) {
-  Label out(this), throw_exception(this, Label::kDeferred);
-  VARIABLE(var_value_map, MachineRepresentation::kTagged);
-
-  GotoIf(TaggedIsSmi(value), &throw_exception);
-
-  // Load the instance type of the {value}.
-  var_value_map.Bind(LoadMap(value));
-  Node* const value_instance_type = LoadMapInstanceType(var_value_map.value());
-
-  Branch(IsJSReceiverInstanceType(value_instance_type), &out, &throw_exception);
-
-  // The {value} is not a compatible receiver for this method.
-  BIND(&throw_exception);
-  ThrowTypeError(context, msg_template, method_name);
-
-  BIND(&out);
-  return var_value_map.value();
-}
-
 Node* PromiseBuiltinsAssembler::PromiseHasHandler(Node* promise) {
   Node* const flags = LoadObjectField(promise, JSPromise::kFlagsOffset);
   return IsSetWord(SmiUntag(flags), 1 << JSPromise::kHasHandlerBit);

src/builtins/builtins-promise-gen.h

@@ -112,10 +112,6 @@ class PromiseBuiltinsAssembler : public CodeStubAssembler {
  protected:
   void PromiseInit(Node* promise);
 
-  Node* ThrowIfNotJSReceiver(Node* context, Node* value,
-                             MessageTemplate::Template msg_template,
-                             const char* method_name = nullptr);
-
   Node* SpeciesConstructor(Node* context, Node* object,
                            Node* default_constructor);
 

src/builtins/builtins-reflect-gen.cc

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/builtins/builtins-utils-gen.h"
+#include "src/builtins/builtins.h"
+#include "src/code-stub-assembler.h"
+
+namespace v8 {
+namespace internal {
+
+// ES section #sec-reflect.has
+TF_BUILTIN(ReflectHas, CodeStubAssembler) {
+  Node* target = Parameter(Descriptor::kTarget);
+  Node* key = Parameter(Descriptor::kKey);
+  Node* context = Parameter(Descriptor::kContext);
+
+  ThrowIfNotJSReceiver(context, target, MessageTemplate::kCalledOnNonObject,
+                       "Reflect.has");
+
+  Return(CallBuiltin(Builtins::kHasProperty, context, key, target));
+}
+
+}  // namespace internal
+}  // namespace v8

src/builtins/builtins-reflect.cc

@@ -138,30 +138,6 @@ BUILTIN(ReflectGetPrototypeOf) {
                            JSReceiver::GetPrototype(isolate, receiver));
 }
 
-// ES6 section 26.1.9 Reflect.has
-BUILTIN(ReflectHas) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(3, args.length());
-  Handle<Object> target = args.at(1);
-  Handle<Object> key = args.at(2);
-
-  if (!target->IsJSReceiver()) {
-    THROW_NEW_ERROR_RETURN_FAILURE(
-        isolate, NewTypeError(MessageTemplate::kCalledOnNonObject,
-                              isolate->factory()->NewStringFromAsciiChecked(
-                                  "Reflect.has")));
-  }
-
-  Handle<Name> name;
-  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, name,
-                                     Object::ToName(isolate, key));
-
-  Maybe<bool> result =
-      JSReceiver::HasProperty(Handle<JSReceiver>::cast(target), name);
-  return result.IsJust() ? *isolate->factory()->ToBoolean(result.FromJust())
-                         : isolate->heap()->exception();
-}
-
 // ES6 section 26.1.10 Reflect.isExtensible
 BUILTIN(ReflectIsExtensible) {
   HandleScope scope(isolate);

src/code-stub-assembler.cc

@@ -3533,6 +3533,28 @@ Node* CodeStubAssembler::ThrowIfNotInstanceType(Node* context, Node* value,
   return var_value_map.value();
 }
 
+Node* CodeStubAssembler::ThrowIfNotJSReceiver(
+    Node* context, Node* value, MessageTemplate::Template msg_template,
+    const char* method_name) {
+  Label out(this), throw_exception(this, Label::kDeferred);
+  VARIABLE(var_value_map, MachineRepresentation::kTagged);
+
+  GotoIf(TaggedIsSmi(value), &throw_exception);
+
+  // Load the instance type of the {value}.
+  var_value_map.Bind(LoadMap(value));
+  Node* const value_instance_type = LoadMapInstanceType(var_value_map.value());
+
+  Branch(IsJSReceiverInstanceType(value_instance_type), &out, &throw_exception);
+
+  // The {value} is not a compatible receiver for this method.
+  BIND(&throw_exception);
+  ThrowTypeError(context, msg_template, method_name);
+
+  BIND(&out);
+  return var_value_map.value();
+}
+
 void CodeStubAssembler::ThrowTypeError(Node* context,
                                        MessageTemplate::Template message,
                                        char const* arg0, char const* arg1) {

src/code-stub-assembler.h

@@ -867,6 +867,11 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
   Node* ThrowIfNotInstanceType(Node* context, Node* value,
                                InstanceType instance_type,
                                char const* method_name);
+  // Throws a TypeError for {method_name} if {value} is not a JSReceiver.
+  // Returns the {value}'s map.
+  Node* ThrowIfNotJSReceiver(Node* context, Node* value,
+                             MessageTemplate::Template msg_template,
+                             const char* method_name = nullptr);
   void ThrowTypeError(Node* context, MessageTemplate::Template message,
                       char const* arg0 = nullptr, char const* arg1 = nullptr);
   void ThrowTypeError(Node* context, MessageTemplate::Template message,

src/compiler/js-call-reducer.cc

@@ -590,6 +590,76 @@ Reduction JSCallReducer::ReduceReflectGetPrototypeOf(Node* node) {
   return ReduceObjectGetPrototype(node, target);
 }
 
+// ES section #sec-reflect.has
+Reduction JSCallReducer::ReduceReflectHas(Node* node) {
+  DCHECK_EQ(IrOpcode::kJSCall, node->opcode());
+  CallParameters const& p = CallParametersOf(node->op());
+  int arity = static_cast<int>(p.arity() - 2);
+  DCHECK_LE(0, arity);
+  Node* target = (arity >= 1) ? NodeProperties::GetValueInput(node, 2)
+                              : jsgraph()->UndefinedConstant();
+  Node* key = (arity >= 2) ? NodeProperties::GetValueInput(node, 3)
+                           : jsgraph()->UndefinedConstant();
+  Node* context = NodeProperties::GetContextInput(node);
+  Node* frame_state = NodeProperties::GetFrameStateInput(node);
+  Node* effect = NodeProperties::GetEffectInput(node);
+  Node* control = NodeProperties::GetControlInput(node);
+
+  // Check whether {target} is a JSReceiver.
+  Node* check = graph()->NewNode(simplified()->ObjectIsReceiver(), target);
+  Node* branch =
+      graph()->NewNode(common()->Branch(BranchHint::kTrue), check, control);
+
+  // Throw an appropriate TypeError if the {target} is not a JSReceiver.
+  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+  Node* efalse = effect;
+  {
+    if_false = efalse = graph()->NewNode(
+        javascript()->CallRuntime(Runtime::kThrowTypeError, 2),
+        jsgraph()->Constant(MessageTemplate::kCalledOnNonObject),
+        jsgraph()->HeapConstant(
+            factory()->NewStringFromAsciiChecked("Reflect.has")),
+        context, frame_state, efalse, if_false);
+  }
+
+  // Otherwise just use the existing {JSHasProperty} logic.
+  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  Node* etrue = effect;
+  Node* vtrue;
+  {
+    vtrue = etrue = if_true =
+        graph()->NewNode(javascript()->HasProperty(), key, target, context,
+                         frame_state, etrue, if_true);
+  }
+
+  // Rewire potential exception edges.
+  Node* on_exception = nullptr;
+  if (NodeProperties::IsExceptionalCall(node, &on_exception)) {
+    // Create appropriate {IfException} and {IfSuccess} nodes.
+    Node* extrue = graph()->NewNode(common()->IfException(), etrue, if_true);
+    if_true = graph()->NewNode(common()->IfSuccess(), if_true);
+    Node* exfalse = graph()->NewNode(common()->IfException(), efalse, if_false);
+    if_false = graph()->NewNode(common()->IfSuccess(), if_false);
+
+    // Join the exception edges.
+    Node* merge = graph()->NewNode(common()->Merge(2), extrue, exfalse);
+    Node* ephi =
+        graph()->NewNode(common()->EffectPhi(2), extrue, exfalse, merge);
+    Node* phi =
+        graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
+                         extrue, exfalse, merge);
+    ReplaceWithValue(on_exception, phi, ephi, merge);
+  }
+
+  // Connect the throwing path to end.
+  if_false = graph()->NewNode(common()->Throw(), efalse, if_false);
+  NodeProperties::MergeControlToEnd(graph(), common(), if_false);
+
+  // Continue on the regular path.
+  ReplaceWithValue(node, vtrue, etrue, if_true);
+  return Changed(vtrue);
+}
+
 bool CanInlineArrayIteratingBuiltin(Handle<Map> receiver_map) {
   Isolate* const isolate = receiver_map->GetIsolate();
   if (!receiver_map->prototype()->IsJSArray()) return false;
@@ -1436,6 +1506,8 @@ Reduction JSCallReducer::ReduceJSCall(Node* node) {
           return ReduceReflectConstruct(node);
         case Builtins::kReflectGetPrototypeOf:
           return ReduceReflectGetPrototypeOf(node);
+        case Builtins::kReflectHas:
+          return ReduceReflectHas(node);
         case Builtins::kArrayForEach:
           return ReduceArrayForEach(function, node);
         case Builtins::kArrayMap:

src/compiler/js-call-reducer.h

@@ -69,6 +69,7 @@ class JSCallReducer final : public AdvancedReducer {
   Reduction ReduceReflectApply(Node* node);
   Reduction ReduceReflectConstruct(Node* node);
   Reduction ReduceReflectGetPrototypeOf(Node* node);
+  Reduction ReduceReflectHas(Node* node);
   Reduction ReduceArrayForEach(Handle<JSFunction> function, Node* node);
   Reduction ReduceArrayMap(Handle<JSFunction> function, Node* node);
   Reduction ReduceCallOrConstructWithArrayLikeOrSpread(

src/v8.gyp

@@ -206,6 +206,7 @@
         'builtins/builtins-promise-gen.h',
         'builtins/builtins-proxy-gen.cc',
         'builtins/builtins-proxy-gen.h',
+        'builtins/builtins-reflect-gen.cc',
         'builtins/builtins-regexp-gen.cc',
         'builtins/builtins-regexp-gen.h',
         'builtins/builtins-sharedarraybuffer-gen.cc',

test/mjsunit/compiler/reflect-has.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+// Test Reflect.has with wrong (number of) arguments.
+(function() {
+  "use strict";
+  function foo() { return Reflect.has(); }
+
+  assertThrows(foo);
+  assertThrows(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo);
+})();
+(function() {
+  "use strict";
+  function foo(o) { return Reflect.has(o); }
+
+  assertFalse(foo({}));
+  assertFalse(foo({}));
+  %OptimizeFunctionOnNextCall(foo);
+  assertFalse(foo({}));
+})();
+(function() {
+  "use strict";
+  function foo(o) { return Reflect.has(o); }
+
+  assertThrows(foo.bind(undefined, 1));
+  assertThrows(foo.bind(undefined, undefined));
+  %OptimizeFunctionOnNextCall(foo);
+  assertThrows(foo.bind(undefined, 'o'));
+})();
+
+// Test Reflect.has within try/catch.
+(function() {
+  "use strict";
+  function foo() {
+    try {
+      return Reflect.has();
+    } catch (e) {
+      return 1;
+    }
+  }
+
+  assertEquals(1, foo());
+  assertEquals(1, foo());
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(1, foo());
+})();
+(function() {
+  "use strict";
+  const o = {};
+  function foo(n) {
+    try {
+      return Reflect.has(o, n);
+    } catch (e) {
+      return 1;
+    }
+  }
+
+  assertEquals(1, foo({[Symbol.toPrimitive]() { throw new Error(); }}));
+  assertEquals(1, foo({[Symbol.toPrimitive]() { throw new Error(); }}));
+  %OptimizeFunctionOnNextCall(foo);
+  assertEquals(1, foo({[Symbol.toPrimitive]() { throw new Error(); }}));
+})();