Bug 910782 - SpiderMonkey: Use jsbytecode instead of int for the switch mask variables, and name the magic opcode used to enable interrupts. r=luke
authorDan Gohman <sunfish@google.com>
Wed, 04 Sep 2013 20:06:07 -0700
changeset 145626 0e876da27431beceee6027b3cc4ece46086189b0
parent 145625 7792dc26b3e1c71c02072295966d88c9c4b24e2e
child 145627 bf286f1d5489d2b2985cccfbf32f917124058e2f
push id25216
push useremorley@mozilla.com
push dateThu, 05 Sep 2013 10:06:42 +0000
treeherdermozilla-central@676322e0166c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs910782
milestone26.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 910782 - SpiderMonkey: Use jsbytecode instead of int for the switch mask variables, and name the magic opcode used to enable interrupts. r=luke
js/src/vm/Interpreter.cpp
js/src/vm/Stack-inl.h
js/src/vm/Stack.h
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1256,31 +1256,37 @@ static JS_NEVER_INLINE bool
 Interpret(JSContext *cx, RunState &state)
 {
     JSAutoResolveFlags rf(cx, RESOLVE_INFER);
 
     gc::MaybeVerifyBarriers(cx, true);
 
     JS_ASSERT(!cx->compartment()->activeAnalysis);
 
-#define CHECK_PCCOUNT_INTERRUPTS() JS_ASSERT_IF(script->hasScriptCounts, switchMask == -1)
+#define CHECK_PCCOUNT_INTERRUPTS() \
+    JS_ASSERT_IF(script->hasScriptCounts, switchMask == EnableInterruptsPseudoOpcode)
 
     /*
      * When Debugger puts a script in single-step mode, all js::Interpret
      * invocations that might be presently running that script must have
      * interrupts enabled. It's not practical to simply check
      * script->stepModeEnabled() at each point some callee could have changed
      * it, because there are so many places js::Interpret could possibly cause
      * JavaScript to run: each place an object might be coerced to a primitive
      * or a number, for example. So instead, we expose a simple mechanism to
      * let Debugger tweak the affected js::Interpret frames when an onStep
-     * handler is added: setting switchMask to -1 will enable interrupts.
+     * handler is added: setting switchMask to EnableInterruptsPseudoOpcode
+     * will enable interrupts.
      */
-    register int switchMask = 0;
-    int switchOp;
+    static_assert(EnableInterruptsPseudoOpcode >= JSOP_LIMIT,
+                  "EnableInterruptsPseudoOpcode must be greater than any opcode");
+    static_assert(EnableInterruptsPseudoOpcode == jsbytecode(-1),
+                  "EnableInterruptsPseudoOpcode must be the maximum jsbytecode value");
+    jsbytecode switchMask = 0;
+    jsbytecode switchOp;
 
 #define DO_OP()            goto do_op
 
 #define BEGIN_CASE(OP)     case OP:
 #define END_CASE(OP)                                                          \
     JS_BEGIN_MACRO                                                            \
         len = OP##_LENGTH;                                                    \
         goto advanceAndDoOp;                                                  \
@@ -1310,17 +1316,17 @@ Interpret(JSContext *cx, RunState &state
             CHECK_BRANCH();                                                   \
         DO_OP();                                                              \
     JS_END_MACRO
 
 #define SET_SCRIPT(s)                                                         \
     JS_BEGIN_MACRO                                                            \
         script = (s);                                                         \
         if (script->hasAnyBreakpointsOrStepMode() || script->hasScriptCounts) \
-            switchMask = -1; /* Enable interrupts. */                         \
+            switchMask = EnableInterruptsPseudoOpcode; /* Enable interrupts. */ \
     JS_END_MACRO
 
     FrameRegs regs;
     FrameGuard fg(state, regs);
 
     StackFrame *entryFrame = state.pushInterpreterFrame(cx, &fg);
     if (!entryFrame)
         return false;
@@ -1409,33 +1415,31 @@ Interpret(JSContext *cx, RunState &state
      * processing of an opcode while looping. We rely on |advanceAndDoOp:| to
      * manage "op" correctly in all other cases.
      */
     JSOp op;
     int32_t len;
     len = 0;
 
     if (rt->profilingScripts || cx->runtime()->debugHooks.interruptHook)
-        switchMask = -1; /* Enable interrupts. */
+        switchMask = EnableInterruptsPseudoOpcode; /* Enable interrupts. */
 
   advanceAndDoOp:
     js::gc::MaybeVerifyBarriers(cx);
     regs.pc += len;
     op = (JSOp) *regs.pc;
 
   do_op:
     CHECK_PCCOUNT_INTERRUPTS();
-    switchOp = int(op) | switchMask;
+    switchOp = jsbytecode(op) | switchMask;
   do_switch:
     switch (switchOp) {
 
-case -1:
+BEGIN_CASE(EnableInterruptsPseudoOpcode)
 {
-    JS_ASSERT(switchMask == -1);
-
     bool moreInterrupts = false;
 
     if (cx->runtime()->profilingScripts) {
         if (!script->hasScriptCounts)
             script->initScriptCounts(cx);
         moreInterrupts = true;
     }
 
@@ -1488,18 +1492,20 @@ case -1:
             goto error;
           default:
             break;
         }
         JS_ASSERT(status == JSTRAP_CONTINUE);
         JS_ASSERT(rval.isInt32() && rval.toInt32() == op);
     }
 
-    switchMask = moreInterrupts ? -1 : 0;
-    switchOp = int(op);
+    JS_ASSERT(switchMask == EnableInterruptsPseudoOpcode);
+    switchMask = moreInterrupts ? EnableInterruptsPseudoOpcode : 0;
+
+    switchOp = jsbytecode(op);
     goto do_switch;
 }
 
 /* Various 1-byte no-ops. */
 BEGIN_CASE(JSOP_NOP)
 BEGIN_CASE(JSOP_UNUSED125)
 BEGIN_CASE(JSOP_UNUSED126)
 BEGIN_CASE(JSOP_UNUSED132)
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -835,17 +835,17 @@ Activation::Activation(JSContext *cx, Ki
 
 Activation::~Activation()
 {
     JS_ASSERT(cx_->mainThread().activation_ == this);
     cx_->mainThread().activation_ = prev_;
 }
 
 InterpreterActivation::InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
-                                             int *const switchMask)
+                                             jsbytecode *const switchMask)
   : Activation(cx, Interpreter),
     entry_(entry),
     regs_(regs),
     switchMask_(switchMask)
 #ifdef DEBUG
   , oldFrameCount_(cx_->runtime()->interpreterStack().frameCount_)
 #endif
 {}
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -1211,33 +1211,38 @@ class Activation
         return savedFrameChain_ > 0;
     }
 
   private:
     Activation(const Activation &other) MOZ_DELETE;
     void operator=(const Activation &other) MOZ_DELETE;
 };
 
+// The value to assign to InterpreterActivation's *switchMask_ to enable
+// interrupts. This value is greater than the greatest opcode, and is chosen
+// such that the bitwise or of this value with any opcode is this value.
+static const jsbytecode EnableInterruptsPseudoOpcode = -1;
+
 class InterpreterFrameIterator;
 
 class InterpreterActivation : public Activation
 {
     friend class js::InterpreterFrameIterator;
 
     StackFrame *const entry_; // Entry frame for this activation.
     FrameRegs &regs_;
-    int *const switchMask_; // For debugger interrupts, see js::Interpret.
+    jsbytecode *const switchMask_; // For debugger interrupts, see js::Interpret.
 
 #ifdef DEBUG
     size_t oldFrameCount_;
 #endif
 
   public:
     inline InterpreterActivation(JSContext *cx, StackFrame *entry, FrameRegs &regs,
-                                 int *const switchMask);
+                                 jsbytecode *const switchMask);
     inline ~InterpreterActivation();
 
     inline bool pushInlineFrame(const CallArgs &args, HandleScript script,
                                 InitialFrameFlags initial);
     inline void popInlineFrame(StackFrame *frame);
 
     StackFrame *current() const {
         return regs_.fp();
@@ -1247,17 +1252,17 @@ class InterpreterActivation : public Act
     }
 
     // If this js::Interpret frame is running |script|, enable interrupts.
     void enableInterruptsIfRunning(JSScript *script) {
         if (regs_.fp()->script() == script)
             enableInterruptsUnconditionally();
     }
     void enableInterruptsUnconditionally() {
-        *switchMask_ = -1;
+        *switchMask_ = EnableInterruptsPseudoOpcode;
     }
 };
 
 // Iterates over a runtime's activation list.
 class ActivationIterator
 {
     uint8_t *jitTop_;