Bug 1566141 - make interpreter work with JSOP code for coalesce r=jorendorff,jandem
authoryulia <ystartsev@mozilla.com>
Tue, 12 Nov 2019 17:33:55 +0000
changeset 501726 9b150dd56999e1c2b7aa47cf9c0e8d96c6c8eff3
parent 501725 eec2b47cdc07333154fc07c0576d200076d4f3de
child 501727 2a8c35575189a16dcdf6af014a12b046817b28b2
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, jandem
bugs1566141
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1566141 - make interpreter work with JSOP code for coalesce r=jorendorff,jandem Differential Revision: https://phabricator.services.mozilla.com/D51638
js/src/debugger/Script.cpp
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/jit/BaselineCodeGen.h
js/src/tests/non262/reflect-parse/basicBuilder.js
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/debugger/Script.cpp
+++ b/js/src/debugger/Script.cpp
@@ -1505,16 +1505,17 @@ static bool BytecodeIsEffectful(JSOp op)
     case JSOP_LABEL:
     case JSOP_UNDEFINED:
     case JSOP_IFNE:
     case JSOP_IFEQ:
     case JSOP_RETURN:
     case JSOP_RETRVAL:
     case JSOP_AND:
     case JSOP_OR:
+    case JSOP_COALESCE:
     case JSOP_TRY:
     case JSOP_THROW:
     case JSOP_GOTO:
     case JSOP_CONDSWITCH:
     case JSOP_TABLESWITCH:
     case JSOP_CASE:
     case JSOP_DEFAULT:
     case JSOP_BITNOT:
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -1244,18 +1244,18 @@ restart:
       return true;
 
     case ParseNodeKind::SetThis:
       MOZ_ASSERT(pn->is<BinaryNode>());
       *answer = true;
       return true;
 
     case ParseNodeKind::StatementList:
-    // Strict equality operations and logical operators are well-behaved and
-    // perform no conversions.
+    // Strict equality operations and short circuit operators are well-behaved
+    // and perform no conversions.
     case ParseNodeKind::CoalesceExpr:
     case ParseNodeKind::OrExpr:
     case ParseNodeKind::AndExpr:
     case ParseNodeKind::StrictEqExpr:
     case ParseNodeKind::StrictNeExpr:
     // Any subexpression of a comma expression could be effectful.
     case ParseNodeKind::CommaExpr:
       MOZ_ASSERT(!pn->as<ListNode>().empty());
@@ -7512,21 +7512,21 @@ bool BytecodeEmitter::emitCallOrNew(
 
 // This list must be kept in the same order in several places:
 //   - The binary operators in ParseNode.h ,
 //   - the binary operators in TokenKind.h
 //   - the precedence list in Parser.cpp
 static const JSOp ParseNodeKindToJSOp[] = {
     // JSOP_NOP is for pipeline operator which does not emit its own JSOp
     // but has highest precedence in binary operators
-    JSOP_NOP,        JSOP_NOP,    JSOP_OR,       JSOP_AND, JSOP_BITOR,
-    JSOP_BITXOR,     JSOP_BITAND, JSOP_STRICTEQ, JSOP_EQ,  JSOP_STRICTNE,
-    JSOP_NE,         JSOP_LT,     JSOP_LE,       JSOP_GT,  JSOP_GE,
-    JSOP_INSTANCEOF, JSOP_IN,     JSOP_LSH,      JSOP_RSH, JSOP_URSH,
-    JSOP_ADD,        JSOP_SUB,    JSOP_MUL,      JSOP_DIV, JSOP_MOD,
+    JSOP_NOP,        JSOP_COALESCE, JSOP_OR,       JSOP_AND, JSOP_BITOR,
+    JSOP_BITXOR,     JSOP_BITAND,   JSOP_STRICTEQ, JSOP_EQ,  JSOP_STRICTNE,
+    JSOP_NE,         JSOP_LT,       JSOP_LE,       JSOP_GT,  JSOP_GE,
+    JSOP_INSTANCEOF, JSOP_IN,       JSOP_LSH,      JSOP_RSH, JSOP_URSH,
+    JSOP_ADD,        JSOP_SUB,      JSOP_MUL,      JSOP_DIV, JSOP_MOD,
     JSOP_POW};
 
 static inline JSOp BinaryOpParseNodeKindToJSOp(ParseNodeKind pnk) {
   MOZ_ASSERT(pnk >= ParseNodeKind::BinOpFirst);
   MOZ_ASSERT(pnk <= ParseNodeKind::BinOpLast);
   int parseNodeFirst = size_t(ParseNodeKind::BinOpFirst);
 #ifdef DEBUG
   int jsopArraySize = ArrayLength(ParseNodeKindToJSOp);
@@ -7568,81 +7568,19 @@ bool BytecodeEmitter::emitLeftAssociativ
     }
     if (!emit1(op)) {
       return false;
     }
   } while ((nextExpr = nextExpr->pn_next));
   return true;
 }
 
-bool BytecodeEmitter::emitNullCoalesce(ListNode* node) {
-  MOZ_ASSERT(node->isKind(ParseNodeKind::CoalesceExpr));
-
-  /*
-   * CoalesceExpr converts the operand on the stack to boolean depending on an
-   * equality check for undefined and null. If true, it leaves the original
-   * value on the stack and jumps; otherwise it falls into the next bytecode,
-   * which pops the left operand and then evaluates the right operand.
-   * The jump goes around the right operand evaluation.
-   */
-
-  TDZCheckCache tdzCache(this);
-
-  JumpList jump;
-  /* Left-associative operator chain: avoid too much recursion. */
-  for (ParseNode* expr = node->head();; expr = expr->pn_next) {
-    if (!emitTree(expr)) {
-      //            [stack] LHS
-      return false;
-    }
-
-    // if there are no nodes after this, break so that we don't emit
-    // unnecessary bytecode instructions
-    if (!expr->pn_next) {
-      break;
-    }
-
-    if (!emitPushNotUndefinedOrNull()) {
-      //            [stack] LHS NOT-UNDEF-OR-NULL
-      return false;
-    }
-
-    // We are using JSOP_IFEQ, so we need to invert the boolean
-    // pushed onto the stack by emitPushNotUndefinedOrNull.
-    // This is to address a constraint in Ion Monkey which throws
-    // if JSOP_IFNE is encountered.
-    if (!emit1(JSOP_NOT)) {
-      //              [stack] LHS UNDEF-OR-NULL
-      return false;
-    }
-
-    // Emit an annotated branch-if-false around the then part.
-    if(!this->newSrcNote(SRC_IF)) {
-      return false;
-    }
-
-    if (!emitJump(JSOP_IFEQ, &jump)) {
-      //              [stack] LHS
-      return false;
-    }
-
-    if (!emit1(JSOP_POP)) {
-      return false;
-    }
-  }
-
-  if (!emitJumpTargetAndPatch(jump)) {
-    return false;
-  }
-
-  return true;
-}
-
-bool BytecodeEmitter::emitLogical(ListNode* node) {
+bool BytecodeEmitter::emitShortCircuit(ListNode* node) {
   MOZ_ASSERT(node->isKind(ParseNodeKind::OrExpr) ||
+             node->isKind(ParseNodeKind::CoalesceExpr) ||
              node->isKind(ParseNodeKind::AndExpr));
 
   /*
    * JSOP_OR converts the operand on the stack to boolean, leaves the original
    * value on the stack and jumps if true; otherwise it falls into the next
    * bytecode, which pops the left operand and then evaluates the right operand.
    * The jump goes around the right operand evaluation.
    *
@@ -7654,17 +7592,31 @@ bool BytecodeEmitter::emitLogical(ListNo
 
   /* Left-associative operator chain: avoid too much recursion. */
   ParseNode* expr = node->head();
 
   if (!emitTree(expr)) {
     return false;
   }
 
-  JSOp op = node->isKind(ParseNodeKind::OrExpr) ? JSOP_OR : JSOP_AND;
+  JSOp op;
+  switch (node->getKind()) {
+    case ParseNodeKind::OrExpr:
+      op = JSOP_OR;
+      break;
+    case ParseNodeKind::CoalesceExpr:
+      op = JSOP_COALESCE;
+      break;
+    case ParseNodeKind::AndExpr:
+      op = JSOP_AND;
+      break;
+    default:
+      MOZ_CRASH("Unexpected ParseNodeKind");
+  }
+
   JumpList jump;
   if (!emitJump(op, &jump)) {
     return false;
   }
   if (!emit1(JSOP_POP)) {
     return false;
   }
 
@@ -9304,25 +9256,20 @@ bool BytecodeEmitter::emitTree(
 
     case ParseNodeKind::ConditionalExpr:
       if (!emitConditionalExpression(pn->as<ConditionalExpression>(),
                                      valueUsage)) {
         return false;
       }
       break;
 
+    case ParseNodeKind::OrExpr:
     case ParseNodeKind::CoalesceExpr:
-      if (!emitNullCoalesce(&pn->as<ListNode>())) {
-        return false;
-      }
-      break;
-
-    case ParseNodeKind::OrExpr:
     case ParseNodeKind::AndExpr:
-      if (!emitLogical(&pn->as<ListNode>())) {
+      if (!emitShortCircuit(&pn->as<ListNode>())) {
         return false;
       }
       break;
 
     case ParseNodeKind::AddExpr:
     case ParseNodeKind::SubExpr:
     case ParseNodeKind::BitOrExpr:
     case ParseNodeKind::BitXorExpr:
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -684,18 +684,17 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   MOZ_MUST_USE bool emitDeleteExpression(UnaryNode* deleteNode);
 
   // |op| must be JSOP_TYPEOF or JSOP_TYPEOFEXPR.
   MOZ_MUST_USE bool emitTypeof(UnaryNode* typeofNode, JSOp op);
 
   MOZ_MUST_USE bool emitUnary(UnaryNode* unaryNode);
   MOZ_MUST_USE bool emitRightAssociative(ListNode* node);
   MOZ_MUST_USE bool emitLeftAssociative(ListNode* node);
-  MOZ_MUST_USE bool emitNullCoalesce(ListNode* node);
-  MOZ_MUST_USE bool emitLogical(ListNode* node);
+  MOZ_MUST_USE bool emitShortCircuit(ListNode* node);
   MOZ_MUST_USE bool emitSequenceExpr(
       ListNode* node, ValueUsage valueUsage = ValueUsage::WantValue);
 
   MOZ_NEVER_INLINE MOZ_MUST_USE bool emitIncOrDec(UnaryNode* incDec);
 
   MOZ_MUST_USE bool emitConditionalExpression(
       ConditionalExpression& conditional,
       ValueUsage valueUsage = ValueUsage::WantValue);
--- a/js/src/jit/BaselineCodeGen.h
+++ b/js/src/jit/BaselineCodeGen.h
@@ -461,16 +461,17 @@ class BaselineCodeGen {
   // firstResumeIndex stored in JSOP_TABLESWITCH.
   void emitTableSwitchJump(Register key, Register scratch1, Register scratch2);
 
   MOZ_MUST_USE bool emitReturn();
 
   MOZ_MUST_USE bool emitToBoolean();
   MOZ_MUST_USE bool emitTest(bool branchIfTrue);
   MOZ_MUST_USE bool emitAndOr(bool branchIfTrue);
+  MOZ_MUST_USE bool emitCoalesce();
 
   MOZ_MUST_USE bool emitCall(JSOp op);
   MOZ_MUST_USE bool emitSpreadCall(JSOp op);
 
   MOZ_MUST_USE bool emitDelElem(bool strict);
   MOZ_MUST_USE bool emitDelProp(bool strict);
   MOZ_MUST_USE bool emitSetElemSuper(bool strict);
   MOZ_MUST_USE bool emitSetPropSuper(bool strict);
--- a/js/src/tests/non262/reflect-parse/basicBuilder.js
+++ b/js/src/tests/non262/reflect-parse/basicBuilder.js
@@ -1,10 +1,10 @@
 // |reftest| skip-if(!xulRuntime.shell)
-function test() { 
+function test() {
 // Builder tests
 
 Pattern("program").match(Reflect.parse("42", {builder:{program:() => "program"}}));
 
 assertGlobalStmt("throw 42", 1, { throwStatement: () => 1 });
 assertGlobalStmt("for (;;);", 2, { forStatement: () => 2 });
 assertGlobalStmt("for (x in y);", 3, { forInStatement: () => 3 });
 assertGlobalStmt("{ }", 4, { blockStatement: () => 4 });
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -2189,16 +2189,25 @@ static MOZ_NEVER_INLINE JS_HAZ_JSNATIVE_
     CASE(JSOP_OR) {
       bool cond = ToBoolean(REGS.stackHandleAt(-1));
       if (cond) {
         ADVANCE_AND_DISPATCH(GET_JUMP_OFFSET(REGS.pc));
       }
     }
     END_CASE(JSOP_OR)
 
+    CASE(JSOP_COALESCE) {
+      MutableHandleValue res = REGS.stackHandleAt(-1);
+      bool cond = !res.isNullOrUndefined();
+      if (cond) {
+        ADVANCE_AND_DISPATCH(GET_JUMP_OFFSET(REGS.pc));
+      }
+    }
+    END_CASE(JSOP_COALESCE)
+
     CASE(JSOP_AND) {
       bool cond = ToBoolean(REGS.stackHandleAt(-1));
       if (!cond) {
         ADVANCE_AND_DISPATCH(GET_JUMP_OFFSET(REGS.pc));
       }
     }
     END_CASE(JSOP_AND)
 
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2563,25 +2563,35 @@
      */ \
     MACRO(JSOP_INSTRUMENTATION_CALLBACK, 239, "instrumentationCallback", NULL, 1, 0, 1, JOF_BYTE) \
     /*
      * Pushes the current script's instrumentation ID.
      *   Category: Other
      *   Operands:
      *   Stack: => val
      */ \
-    MACRO(JSOP_INSTRUMENTATION_SCRIPT_ID, 240, "instrumentationScriptId", NULL, 1, 0, 1, JOF_BYTE)
+    MACRO(JSOP_INSTRUMENTATION_SCRIPT_ID, 240, "instrumentationScriptId", NULL, 1, 0, 1, JOF_BYTE) \
+    /*
+     * If the value on top of the stack is not null or undefined, jumps to a 32-bit offset from the
+     * current bytecode.
+     *
+     *   Category: Statements
+     *   Type: Jumps
+     *   Operands: int32_t offset
+     *   Stack: cond => cond
+     */ \
+    MACRO(JSOP_COALESCE, 241, "coalesce", NULL, 5, 1, 1, JOF_JUMP|JOF_DETECTING)
+
 // clang-format on
 
 /*
  * In certain circumstances it may be useful to "pad out" the opcode space to
  * a power of two.  Use this macro to do so.
  */
 #define FOR_EACH_TRAILING_UNUSED_OPCODE(MACRO) \
-  MACRO(241)                                   \
   MACRO(242)                                   \
   MACRO(243)                                   \
   MACRO(244)                                   \
   MACRO(245)                                   \
   MACRO(246)                                   \
   MACRO(247)                                   \
   MACRO(248)                                   \
   MACRO(249)                                   \