[regexp] Address various minor nits

src/builtins/builtins-regexp-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-regexp-gen.h"
 
 #include "src/builtins/builtins-constructor-gen.h"
 #include "src/builtins/builtins-utils-gen.h"
 #include "src/builtins/builtins.h"
 #include "src/code-factory.h"
@@ skipping 82 matching lines @@
 
   StoreFixedArrayElement(result_elements, 0, first, SKIP_WRITE_BARRIER);
 
   // If no captures exist we can skip named capture handling as well.
   GotoIf(SmiEqual(num_results, SmiConstant(1)), &out);
 
   // Store all remaining captures.
   Node* const limit = IntPtrAdd(
       IntPtrConstant(RegExpMatchInfo::kFirstCaptureIndex), num_indices);
 
-  Variable var_from_cursor(
-      this, MachineType::PointerRepresentation(),
-      IntPtrConstant(RegExpMatchInfo::kFirstCaptureIndex + 2));
+  VARIABLE(var_from_cursor, MachineType::PointerRepresentation(),
+           IntPtrConstant(RegExpMatchInfo::kFirstCaptureIndex + 2));
   VARIABLE(var_to_cursor, MachineType::PointerRepresentation(),
            IntPtrConstant(1));
 
   Variable* vars[] = {&var_from_cursor, &var_to_cursor};
   Label loop(this, 2, vars);
 
   Goto(&loop);
   BIND(&loop);
   {
     Node* const from_cursor = var_from_cursor.value();
@@ skipping 363 matching lines @@
 // match) or a fixed array containing match indices as returned by
 // RegExpExecStub.
 Node* RegExpBuiltinsAssembler::RegExpPrototypeExecBodyWithoutResult(
     Node* const context, Node* const regexp, Node* const string,
     Label* if_didnotmatch, const bool is_fastpath) {
   Node* const null = NullConstant();
   Node* const int_zero = IntPtrConstant(0);
   Node* const smi_zero = SmiConstant(Smi::kZero);
 
   if (is_fastpath) {
-    CSA_ASSERT(this, IsFastRegExpNoPrototype(context, regexp, LoadMap(regexp)));
+    CSA_ASSERT(this, IsFastRegExpNoPrototype(context, regexp));
   } else {
     ThrowIfNotInstanceType(context, regexp, JS_REGEXP_TYPE,
                            "RegExp.prototype.exec");
   }
 
   CSA_ASSERT(this, IsString(string));
   CSA_ASSERT(this, IsJSRegExp(regexp));
 
   VARIABLE(var_result, MachineRepresentation::kTagged);
   Label out(this);
@@ skipping 187 matching lines @@
   // The smi check is required to omit ToLength(lastIndex) calls with possible
   // user-code execution on the fast path.
   Node* const last_index = FastLoadLastIndex(object);
   var_result.Bind(TaggedIsPositiveSmi(last_index));
   Goto(&out);
 
   BIND(&out);
   return var_result.value();
 }
 
+Node* RegExpBuiltinsAssembler::IsFastRegExpNoPrototype(Node* const context,
+                                                       Node* const object) {
+  CSA_ASSERT(this, TaggedIsNotSmi(object));

[Camillo Bruni, 2017/04/11 09:21:13]

Probably even: IsJSReceiver(object)

[jgruber, 2017/04/11 09:57:24]

All I want to assert here is that we can load a map from the object, so
TaggedIsNotSmi seems more appropriate.

+  return IsFastRegExpNoPrototype(context, object, LoadMap(object));
+}
+
 // RegExp fast path implementations rely on unmodified JSRegExp instances.
 // We use a fairly coarse granularity for this and simply check whether both
 // the regexp itself is unmodified (i.e. its map has not changed), its
 // prototype is unmodified, and lastIndex is a non-negative smi.
 void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
                                                  Node* const object,
                                                  Node* const map,
                                                  Label* const if_isunmodified,
                                                  Label* const if_ismodified) {
   CSA_ASSERT(this, WordEqual(LoadMap(object), map));
@@ skipping 17 matching lines @@
       WordEqual(proto_map, initial_proto_initial_map);
 
   GotoIfNot(proto_has_initialmap, if_ismodified);
 
   // The smi check is required to omit ToLength(lastIndex) calls with possible
   // user-code execution on the fast path.
   Node* const last_index = FastLoadLastIndex(object);
   Branch(TaggedIsPositiveSmi(last_index), if_isunmodified, if_ismodified);
 }
 
+void RegExpBuiltinsAssembler::BranchIfFastRegExp(Node* const context,
+                                                 Node* const object,
+                                                 Label* const if_isunmodified,
+                                                 Label* const if_ismodified) {
+  CSA_ASSERT(this, TaggedIsNotSmi(object));

[Camillo Bruni, 2017/04/11 09:21:13]

same here

+  BranchIfFastRegExp(context, object, LoadMap(object), if_isunmodified,
+                     if_ismodified);
+}
+
 Node* RegExpBuiltinsAssembler::IsFastRegExp(Node* const context,
                                             Node* const object) {
   Label yup(this), nope(this), out(this);
   VARIABLE(var_result, MachineRepresentation::kWord32);
 
-  BranchIfFastRegExp(context, object, LoadMap(object), &yup, &nope);
+  BranchIfFastRegExp(context, object, &yup, &nope);
 
   BIND(&yup);
   var_result.Bind(Int32Constant(1));
   Goto(&out);
 
   BIND(&nope);
   var_result.Bind(Int32Constant(0));
   Goto(&out);
 
   BIND(&out);
   return var_result.value();
 }
 
-void RegExpBuiltinsAssembler::BranchIfFastRegExpResult(Node* context, Node* map,
+void RegExpBuiltinsAssembler::BranchIfFastRegExpResult(Node* const context,
+                                                       Node* const object,
                                                        Label* if_isunmodified,
                                                        Label* if_ismodified) {
+  // Could be a Smi.
+  Node* const map = LoadReceiverMap(object);
+
   Node* const native_context = LoadNativeContext(context);
   Node* const initial_regexp_result_map =
       LoadContextElement(native_context, Context::REGEXP_RESULT_MAP_INDEX);
 
   Branch(WordEqual(map, initial_regexp_result_map), if_isunmodified,
          if_ismodified);
 }
 
 // Slow path stub for RegExpPrototypeExec to decrease code size.
 TF_BUILTIN(RegExpPrototypeExecSlow, RegExpBuiltinsAssembler) {
@@ skipping 13 matching lines @@
 
   // Ensure {maybe_receiver} is a JSRegExp.
   ThrowIfNotInstanceType(context, maybe_receiver, JS_REGEXP_TYPE,
                          "RegExp.prototype.exec");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = ToString(context, maybe_string);
 
   Label if_isfastpath(this), if_isslowpath(this);
-  Branch(IsFastRegExpNoPrototype(context, receiver, LoadMap(receiver)),
-         &if_isfastpath, &if_isslowpath);
+  Branch(IsFastRegExpNoPrototype(context, receiver), &if_isfastpath,
+         &if_isslowpath);
 
   BIND(&if_isfastpath);
   {
     Node* const result =
         RegExpPrototypeExecBody(context, receiver, string, true);
     Return(result);
   }
 
   BIND(&if_isslowpath);
   {
@@ skipping 82 matching lines @@
 #undef CASE_FOR_FLAG
   }
 
   // Allocate a string of the required length and fill it with the corresponding
   // char for each set flag.
 
   {
     Node* const result = AllocateSeqOneByteString(context, var_length.value());
     Node* const flags_intptr = var_flags.value();
 
-    Variable var_offset(
-        this, MachineType::PointerRepresentation(),
-        IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag));
+    VARIABLE(var_offset, MachineType::PointerRepresentation(),
+             IntPtrConstant(SeqOneByteString::kHeaderSize - kHeapObjectTag));
 
 #define CASE_FOR_FLAG(FLAG, CHAR)                              \
   do {                                                         \
     Label next(this);                                          \
     GotoIfNot(IsSetWord(flags_intptr, FLAG), &next);           \
     Node* const value = Int32Constant(CHAR);                   \
     StoreNoWriteBarrier(MachineRepresentation::kWord8, result, \
                         var_offset.value(), value);            \
     var_offset.Bind(IntPtrAdd(var_offset.value(), int_one));   \
     Goto(&next);                                               \
@@ skipping 588 matching lines @@
   // Ensure {maybe_receiver} is a JSReceiver.
   ThrowIfNotJSReceiver(context, maybe_receiver,
                        MessageTemplate::kIncompatibleMethodReceiver,
                        "RegExp.prototype.test");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = ToString(context, maybe_string);
 
   Label fast_path(this), slow_path(this);
-  BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
-                     &slow_path);
+  BranchIfFastRegExp(context, receiver, &fast_path, &slow_path);
 
   BIND(&fast_path);
   {
     Label if_didnotmatch(this);
     RegExpPrototypeExecBodyWithoutResult(context, receiver, string,
                                          &if_didnotmatch, true);
     Return(TrueConstant());
 
     BIND(&if_didnotmatch);
     Return(FalseConstant());
@@ skipping 293 matching lines @@
       } else {
         DCHECK(!is_fastpath);
         Node* const result = RegExpExec(context, regexp, string);
 
         Label load_match(this);
         Branch(WordEqual(result, null), &if_didnotmatch, &load_match);
 
         BIND(&load_match);
         {
           Label fast_result(this), slow_result(this);
-          BranchIfFastRegExpResult(context, LoadMap(result), &fast_result,
-                                   &slow_result);
+          BranchIfFastRegExpResult(context, result, &fast_result, &slow_result);
 
           BIND(&fast_result);
           {
             Node* const result_fixed_array = LoadElements(result);
             Node* const match = LoadFixedArrayElement(result_fixed_array, 0);
 
             // The match is guaranteed to be a string on the fast path.
             CSA_ASSERT(this, IsStringInstanceType(LoadInstanceType(match)));
 
             var_match.Bind(match);
@@ skipping 74 matching lines @@
   // Ensure {maybe_receiver} is a JSReceiver.
   ThrowIfNotJSReceiver(context, maybe_receiver,
                        MessageTemplate::kIncompatibleMethodReceiver,
                        "RegExp.prototype.@@match");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = ToString(context, maybe_string);
 
   Label fast_path(this), slow_path(this);
-  BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
-                     &slow_path);
+  BranchIfFastRegExp(context, receiver, &fast_path, &slow_path);
 
   BIND(&fast_path);
   RegExpPrototypeMatchBody(context, receiver, string, true);
 
   BIND(&slow_path);
   RegExpPrototypeMatchBody(context, receiver, string, false);
 }
 
 void RegExpBuiltinsAssembler::RegExpPrototypeSearchBodyFast(
     Node* const context, Node* const regexp, Node* const string) {
@@ skipping 72 matching lines @@
   {
     Label next(this);
     GotoIfNot(WordEqual(exec_result, NullConstant()), &next);
     Return(SmiConstant(-1));
     BIND(&next);
   }
 
   // Return the index of the match.
   {
     Label fast_result(this), slow_result(this, Label::kDeferred);
-    BranchIfFastRegExpResult(context, LoadMap(exec_result), &fast_result,
-                             &slow_result);
+    BranchIfFastRegExpResult(context, exec_result, &fast_result, &slow_result);
 
     BIND(&fast_result);
     {
       Node* const index =
           LoadObjectField(exec_result, JSRegExpResult::kIndexOffset);
       Return(index);
     }
 
     BIND(&slow_result);
     {
@@ skipping 13 matching lines @@
   // Ensure {maybe_receiver} is a JSReceiver.
   ThrowIfNotJSReceiver(context, maybe_receiver,
                        MessageTemplate::kIncompatibleMethodReceiver,
                        "RegExp.prototype.@@search");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = ToString(context, maybe_string);
 
   Label fast_path(this), slow_path(this);
-  BranchIfFastRegExp(context, receiver, LoadMap(receiver), &fast_path,
-                     &slow_path);
+  BranchIfFastRegExp(context, receiver, &fast_path, &slow_path);
 
   BIND(&fast_path);
   RegExpPrototypeSearchBodyFast(context, receiver, string);
 
   BIND(&slow_path);
   RegExpPrototypeSearchBodySlow(context, receiver, string);
 }
 
 // Generates the fast path for @@split. {regexp} is an unmodified JSRegExp,
 // {string} is a String, and {limit} is a Smi.
@@ skipping 265 matching lines @@
   GotoIf(IsUndefined(maybe_limit), &if_limitissmimax);
   GotoIf(TaggedIsPositiveSmi(maybe_limit), &limit_done);
 
   Node* const limit = ToUint32(context, maybe_limit);
   {
     // ToUint32(limit) could potentially change the shape of the RegExp
     // object. Recheck that we are still on the fast path and bail to runtime
     // otherwise.
     {
       Label next(this);
-      BranchIfFastRegExp(context, regexp, LoadMap(regexp), &next, &runtime);
-      Bind(&next);
+      BranchIfFastRegExp(context, regexp, &next, &runtime);
+      BIND(&next);
     }
 
     GotoIfNot(TaggedIsSmi(limit), &if_limitissmimax);
 
     var_limit.Bind(limit);
     Goto(&limit_done);
   }
 
   BIND(&if_limitissmimax);
   {
     // TODO(jgruber): In this case, we can probably avoid generation of limit
     // checks in Generate_RegExpPrototypeSplitBody.
     var_limit.Bind(SmiConstant(Smi::kMaxValue));
     Goto(&limit_done);
   }
 
   BIND(&limit_done);
   {
     Node* const limit = var_limit.value();
     RegExpPrototypeSplitBody(context, regexp, string, limit);
   }
 
-  Bind(&runtime);
+  BIND(&runtime);
   {
     // The runtime call passes in limit to ensure the second ToUint32(limit)
     // call is not observable.
     CSA_ASSERT(this, IsHeapNumberMap(LoadReceiverMap(limit)));
     Return(CallRuntime(Runtime::kRegExpSplit, context, regexp, string, limit));
   }
 }
 
 // ES#sec-regexp.prototype-@@split
 // RegExp.prototype [ @@split ] ( string, limit )
 TF_BUILTIN(RegExpPrototypeSplit, RegExpBuiltinsAssembler) {
   Node* const maybe_receiver = Parameter(Descriptor::kReceiver);
   Node* const maybe_string = Parameter(Descriptor::kString);
   Node* const maybe_limit = Parameter(Descriptor::kLimit);
   Node* const context = Parameter(Descriptor::kContext);
 
   // Ensure {maybe_receiver} is a JSReceiver.
   ThrowIfNotJSReceiver(context, maybe_receiver,
                        MessageTemplate::kIncompatibleMethodReceiver,
                        "RegExp.prototype.@@split");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = ToString(context, maybe_string);
 
   Label stub(this), runtime(this, Label::kDeferred);
-  BranchIfFastRegExp(context, receiver, LoadMap(receiver), &stub, &runtime);
+  BranchIfFastRegExp(context, receiver, &stub, &runtime);
 
   BIND(&stub);
   Return(CallBuiltin(Builtins::kRegExpSplit, context, receiver, string,
                      maybe_limit));
 
   BIND(&runtime);
   Return(CallRuntime(Runtime::kRegExpSplit, context, receiver, string,
                      maybe_limit));
 }
 
@@ skipping 335 matching lines @@
   BIND(&checkreplacestring);
   {
     Node* const replace_string =
         CallBuiltin(Builtins::kToString, context, replace_value);
 
     // ToString(replaceValue) could potentially change the shape of the RegExp
     // object. Recheck that we are still on the fast path and bail to runtime
     // otherwise.
     {
       Label next(this);
-      BranchIfFastRegExp(context, regexp, LoadMap(regexp), &next, &runtime);
-      Bind(&next);
+      BranchIfFastRegExp(context, regexp, &next, &runtime);
+      BIND(&next);
     }
 
     Node* const dollar_string = HeapConstant(
         isolate()->factory()->LookupSingleCharacterStringFromCode('$'));
     Node* const dollar_ix =
         CallBuiltin(Builtins::kStringIndexOf, context, replace_string,
                     dollar_string, SmiConstant(0));
     GotoIfNot(SmiEqual(dollar_ix, SmiConstant(-1)), &runtime);
 
     Return(
@@ skipping 54 matching lines @@
   ThrowIfNotJSReceiver(context, maybe_receiver,
                        MessageTemplate::kIncompatibleMethodReceiver,
                        "RegExp.prototype.@@replace");
   Node* const receiver = maybe_receiver;
 
   // Convert {maybe_string} to a String.
   Node* const string = CallBuiltin(Builtins::kToString, context, maybe_string);
 
   // Fast-path checks: 1. Is the {receiver} an unmodified JSRegExp instance?
   Label stub(this), runtime(this, Label::kDeferred);
-  BranchIfFastRegExp(context, receiver, LoadMap(receiver), &stub, &runtime);
+  BranchIfFastRegExp(context, receiver, &stub, &runtime);
 
   BIND(&stub);
   Return(CallBuiltin(Builtins::kRegExpReplace, context, receiver, string,
                      replace_value));
 
   BIND(&runtime);
   Return(CallRuntime(Runtime::kRegExpReplace, context, receiver, string,
                      replace_value));
 }
 
@@ skipping 26 matching lines @@
   BIND(&if_matched);
   {
     Node* result =
         ConstructNewResultFromMatchInfo(context, regexp, match_indices, string);
     Return(result);
   }
 }
 
 }  // namespace internal
 }  // namespace v8