Bug 1522837 part 11 - Implement some simple ops in BaselineInterpreterCodeGen. r=djvj
authorJan de Mooij <jdemooij@mozilla.com>
Sun, 10 Mar 2019 19:43:26 +0000
changeset 521298 7a376bfac6dea0340625911e8bb02ad30c21d5e8
parent 521297 cb6108540cef8f493ec19b75517e8d7c70501a59
child 521299 cbba4888990efd52952797d97b60e06dfddc7aac
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1522837
milestone67.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 1522837 part 11 - Implement some simple ops in BaselineInterpreterCodeGen. r=djvj Differential Revision: https://phabricator.services.mozilla.com/D18253
js/src/jit/BaselineCompiler.cpp
js/src/jit/BaselineCompiler.h
js/src/jit/MacroAssembler.h
js/src/jit/arm/MacroAssembler-arm-inl.h
js/src/jit/arm64/MacroAssembler-arm64-inl.h
js/src/jit/x64/MacroAssembler-x64-inl.h
js/src/jit/x86/MacroAssembler-x86-inl.h
js/src/vm/BytecodeUtil.h
js/src/vm/Opcodes.h
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -41,16 +41,23 @@ using namespace js;
 using namespace js::jit;
 
 using mozilla::AssertedCast;
 using mozilla::Maybe;
 
 namespace js {
 namespace jit {
 
+// When generating the Baseline Interpreter, this register is guaranteed to hold
+// the bytecode pc at the start of a bytecode instruction. Instructions are free
+// to clobber this register: the frame's interpreterPC is the canonical
+// location for the pc. This register is useful to avoid loading the pc when
+// compiling simple ops like JSOP_INT8 or JSOP_GETLOCAL.
+static constexpr Register PCRegAtStart = R2.scratchReg();
+
 BaselineCompilerHandler::BaselineCompilerHandler(JSContext* cx,
                                                  MacroAssembler& masm,
                                                  TempAllocator& alloc,
                                                  JSScript* script)
     : frame_(script, masm),
       alloc_(alloc),
       analysis_(alloc, script),
       script_(script),
@@ -366,16 +373,45 @@ MethodStatus BaselineCompiler::compile()
 
 #ifdef MOZ_VTUNE
   vtune::MarkScript(code, script, "baseline");
 #endif
 
   return Method_Compiled;
 }
 
+static void LoadInt8Operand(MacroAssembler& masm, Register pc, Register dest) {
+  masm.load8SignExtend(Address(pc, sizeof(jsbytecode)), dest);
+}
+
+static void LoadUint8Operand(MacroAssembler& masm, Register pc, Register dest) {
+  masm.load8ZeroExtend(Address(pc, sizeof(jsbytecode)), dest);
+}
+
+static void LoadUint16Operand(MacroAssembler& masm, Register pc,
+                              Register dest) {
+  masm.load16ZeroExtend(Address(pc, sizeof(jsbytecode)), dest);
+}
+
+static void LoadInt32Operand(MacroAssembler& masm, Register pc, Register dest) {
+  masm.load32(Address(pc, sizeof(jsbytecode)), dest);
+}
+
+static void LoadInt32OperandSignExtendToPtr(MacroAssembler& masm, Register pc,
+                                            Register dest) {
+  masm.load32SignExtendToPtr(Address(pc, sizeof(jsbytecode)), dest);
+}
+
+static void LoadUint24Operand(MacroAssembler& masm, Register pc, size_t offset,
+                              Register dest) {
+  // Load the opcode and operand, then left shift to discard the opcode.
+  masm.load32(Address(pc, offset), dest);
+  masm.rshift32(Imm32(8), dest);
+}
+
 template <>
 void BaselineCompilerCodeGen::loadScript(Register dest) {
   masm.movePtr(ImmGCPtr(handler.script()), dest);
 }
 
 template <>
 void BaselineInterpreterCodeGen::loadScript(Register dest) {
   masm.loadPtr(frame.addressOfInterpreterScript(), dest);
@@ -1304,17 +1340,19 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_POPN() {
   frame.popn(GET_UINT16(handler.pc()));
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_POPN() {
-  MOZ_CRASH("NYI: interpreter JSOP_POPN");
+  LoadUint16Operand(masm, PCRegAtStart, R0.scratchReg());
+  frame.popn(R0.scratchReg());
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_DUPAT() {
   frame.syncStack(0);
 
   // DUPAT takes a value on the stack and re-pushes it on top.  It's like
   // GETLOCAL but it addresses from the top of the stack instead of from the
@@ -1323,17 +1361,20 @@ bool BaselineCompilerCodeGen::emit_JSOP_
   int depth = -(GET_UINT24(handler.pc()) + 1);
   masm.loadValue(frame.addressOfStackValue(depth), R0);
   frame.push(R0);
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_DUPAT() {
-  MOZ_CRASH("NYI: interpreter JSOP_DUPAT");
+  LoadUint24Operand(masm, PCRegAtStart, 0, R0.scratchReg());
+  masm.loadValue(frame.addressOfStackValue(R0.scratchReg()), R0);
+  frame.push(R0);
+  return true;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_DUP() {
   // Keep top stack value in R0, sync the rest so that we can use R1. We use
   // separate registers because every register can be used by at most one
   // StackValue.
   frame.popRegsAndSync(1);
@@ -1392,31 +1433,53 @@ bool BaselineCompilerCodeGen::emit_JSOP_
   // Push R0.
   frame.pop();
   frame.push(R0);
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_PICK() {
-  MOZ_CRASH("NYI: interpreter JSOP_PICK");
+  // First, move the value to move up into R0.
+  LoadUint8Operand(masm, PCRegAtStart, PCRegAtStart);
+  masm.loadValue(frame.addressOfStackValue(PCRegAtStart), R0);
+
+  // Move the other values down.
+  Label top, done;
+  masm.bind(&top);
+  masm.sub32(Imm32(1), PCRegAtStart);
+  masm.branchTest32(Assembler::Signed, PCRegAtStart, PCRegAtStart, &done);
+  {
+    masm.loadValue(frame.addressOfStackValue(PCRegAtStart), R1);
+    masm.storeValue(R1, frame.addressOfStackValue(PCRegAtStart, sizeof(Value)));
+    masm.jump(&top);
+  }
+
+  masm.bind(&done);
+
+  // Replace value on top of the stack with R0.
+  masm.storeValue(R0, frame.addressOfStackValue(-1));
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_UNPICK() {
   frame.syncStack(0);
 
   // Pick takes the top of the stack value and moves it under the nth value.
   // For instance, unpick 2:
   //     before: A B C D E
   //     after : A B E C D
 
   // First, move value at -1 into R0.
   masm.loadValue(frame.addressOfStackValue(-1), R0);
 
+  MOZ_ASSERT(GET_INT8(handler.pc()) > 0,
+             "Interpreter code assumes JSOP_UNPICK operand > 0");
+
   // Move the other values up.
   int32_t depth = -(GET_INT8(handler.pc()) + 1);
   for (int32_t i = -1; i > depth; i--) {
     Address source = frame.addressOfStackValue(i - 1);
     Address dest = frame.addressOfStackValue(i);
     masm.loadValue(source, R1);
     masm.storeValue(R1, dest);
   }
@@ -1424,33 +1487,82 @@ bool BaselineCompilerCodeGen::emit_JSOP_
   // Store R0 under the nth value.
   Address dest = frame.addressOfStackValue(depth);
   masm.storeValue(R0, dest);
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_UNPICK() {
-  MOZ_CRASH("NYI: interpreter JSOP_UNPICK");
+  LoadUint8Operand(masm, PCRegAtStart, PCRegAtStart);
+
+  // Move the top value into R0.
+  masm.loadValue(frame.addressOfStackValue(-1), R0);
+
+  // Overwrite the nth stack value with R0 but first save the old value in R1.
+  masm.loadValue(frame.addressOfStackValue(PCRegAtStart), R1);
+  masm.storeValue(R0, frame.addressOfStackValue(PCRegAtStart));
+
+  // Now for each slot x in [n-1, 1] do the following:
+  //
+  // * Store the value in slot x in R0.
+  // * Store the value in the previous slot (now in R1) in slot x.
+  // * Move R0 to R1.
+
+#ifdef DEBUG
+  // Assert the operand > 0 so the sub32 below doesn't "underflow" to negative
+  // values.
+  {
+    Label ok;
+    masm.branch32(Assembler::GreaterThan, PCRegAtStart, Imm32(0), &ok);
+    masm.assumeUnreachable("JSOP_UNPICK with operand <= 0?");
+    masm.bind(&ok);
+  }
+#endif
+
+  Label top, done;
+  masm.bind(&top);
+  masm.sub32(Imm32(1), PCRegAtStart);
+  masm.branchTest32(Assembler::Zero, PCRegAtStart, PCRegAtStart, &done);
+  {
+    // Overwrite stack slot x with slot x + 1, saving the old value in R1.
+    masm.loadValue(frame.addressOfStackValue(PCRegAtStart), R0);
+    masm.storeValue(R1, frame.addressOfStackValue(PCRegAtStart));
+    masm.moveValue(R0, R1);
+    masm.jump(&top);
+  }
+
+  // Finally, replace the value on top of the stack (slot 0) with R1. This is
+  // the value that used to be in slot 1.
+  masm.bind(&done);
+  masm.storeValue(R1, frame.addressOfStackValue(-1));
+  return true;
 }
 
 template <>
 void BaselineCompilerCodeGen::emitJump() {
   jsbytecode* pc = handler.pc();
   MOZ_ASSERT(IsJumpOpcode(JSOp(*pc)));
   frame.assertSyncedStack();
 
   jsbytecode* target = pc + GET_JUMP_OFFSET(pc);
   masm.jump(handler.labelOf(target));
 }
 
 template <>
 void BaselineInterpreterCodeGen::emitJump() {
-  // We have to add the current pc's jump offset to the frame's pc.
-  MOZ_CRASH("NYI: interpreter emitJump");
+  // We have to add the current pc's jump offset to the frame's pc. We can use
+  // R0 as scratch because we jump to the "next op" label and that assumes a
+  // synced stack.
+  Register scratch = R0.scratchReg();
+  masm.loadPtr(frame.addressOfInterpreterPC(), scratch);
+  LoadInt32OperandSignExtendToPtr(masm, scratch, scratch);
+  masm.addPtr(frame.addressOfInterpreterPC(), scratch);
+  masm.storePtr(scratch, frame.addressOfInterpreterPC());
+  masm.jump(handler.interpretOpLabel());
 }
 
 template <>
 void BaselineCompilerCodeGen::emitTestBooleanTruthy(bool branchIfTrue,
                                                     ValueOperand val) {
   jsbytecode* pc = handler.pc();
   MOZ_ASSERT(IsJumpOpcode(JSOp(*pc)));
   frame.assertSyncedStack();
@@ -1960,50 +2072,62 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_INT8() {
   frame.push(Int32Value(GET_INT8(handler.pc())));
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_INT8() {
-  MOZ_CRASH("NYI: interpreter JSOP_INT8");
+  LoadInt8Operand(masm, PCRegAtStart, R0.scratchReg());
+  masm.tagValue(JSVAL_TYPE_INT32, R0.scratchReg(), R0);
+  frame.push(R0);
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_INT32() {
   frame.push(Int32Value(GET_INT32(handler.pc())));
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_INT32() {
-  MOZ_CRASH("NYI: interpreter JSOP_INT32");
+  LoadInt32Operand(masm, PCRegAtStart, R0.scratchReg());
+  masm.tagValue(JSVAL_TYPE_INT32, R0.scratchReg(), R0);
+  frame.push(R0);
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_UINT16() {
   frame.push(Int32Value(GET_UINT16(handler.pc())));
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_UINT16() {
-  MOZ_CRASH("NYI: interpreter JSOP_UINT16");
+  LoadUint16Operand(masm, PCRegAtStart, R0.scratchReg());
+  masm.tagValue(JSVAL_TYPE_INT32, R0.scratchReg(), R0);
+  frame.push(R0);
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_UINT24() {
   frame.push(Int32Value(GET_UINT24(handler.pc())));
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_UINT24() {
-  MOZ_CRASH("NYI: interpreter JSOP_UINT24");
+  LoadUint24Operand(masm, PCRegAtStart, 0, R0.scratchReg());
+  masm.tagValue(JSVAL_TYPE_INT32, R0.scratchReg(), R0);
+  frame.push(R0);
+  return true;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emit_JSOP_RESUMEINDEX() {
   return emit_JSOP_UINT24();
 }
 
 template <>
@@ -3500,35 +3624,54 @@ bool BaselineCodeGen<Handler>::emit_JSOP
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_GETLOCAL() {
   frame.pushLocal(GET_LOCALNO(handler.pc()));
   return true;
 }
 
+static BaseValueIndex ComputeAddressOfLocal(MacroAssembler& masm,
+                                            Register indexScratch) {
+  // Locals are stored in memory at a negative offset from the frame pointer. We
+  // negate the index first to effectively subtract it.
+  masm.negPtr(indexScratch);
+  return BaseValueIndex(BaselineFrameReg, indexScratch,
+                        BaselineFrame::reverseOffsetOfLocal(0));
+}
+
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_GETLOCAL() {
-  MOZ_CRASH("NYI: interpreter JSOP_GETLOCAL");
+  Register scratch = R0.scratchReg();
+  LoadUint24Operand(masm, PCRegAtStart, 0, scratch);
+  BaseValueIndex addr = ComputeAddressOfLocal(masm, scratch);
+  masm.loadValue(addr, R0);
+  frame.push(R0);
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emit_JSOP_SETLOCAL() {
   // Ensure no other StackValue refers to the old value, for instance i + (i =
   // 3). This also allows us to use R0 as scratch below.
   frame.syncStack(1);
 
   uint32_t local = GET_LOCALNO(handler.pc());
   frame.storeStackValue(-1, frame.addressOfLocal(local), R0);
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emit_JSOP_SETLOCAL() {
-  MOZ_CRASH("NYI: interpreter JSOP_SETLOCAL");
+  Register scratch = R0.scratchReg();
+  LoadUint24Operand(masm, PCRegAtStart, 0, scratch);
+  BaseValueIndex addr = ComputeAddressOfLocal(masm, scratch);
+  masm.loadValue(frame.addressOfStackValue(-1), R1);
+  masm.storeValue(R1, addr);
+  return true;
 }
 
 template <>
 bool BaselineCompilerCodeGen::emitFormalArgAccess(JSOp op) {
   MOZ_ASSERT(op == JSOP_GETARG || op == JSOP_SETARG);
 
   uint32_t arg = GET_ARGNO(handler.pc());
 
@@ -3792,25 +3935,43 @@ bool BaselineCompilerCodeGen::emitCall(J
   masm.move32(Imm32(argc), R0.scratchReg());
 
   // Call IC
   if (!emitNextIC()) {
     return false;
   }
 
   // Update FrameInfo.
-  bool construct = op == JSOP_NEW || op == JSOP_SUPERCALL;
+  bool construct = IsConstructorCallOp(op);
   frame.popn(2 + argc + construct);
   frame.push(R0);
   return true;
 }
 
 template <>
 bool BaselineInterpreterCodeGen::emitCall(JSOp op) {
-  MOZ_CRASH("NYI: interpreter emitCall");
+  MOZ_ASSERT(IsCallOp(op));
+
+  // The IC expects argc in R0.
+  LoadUint16Operand(masm, PCRegAtStart, R0.scratchReg());
+  if (!emitNextIC()) {
+    return false;
+  }
+
+  // Pop the arguments. We have to reload pc/argc because the IC clobbers them.
+  // The return value is in R0 so we can't use that.
+  Register scratch = R1.scratchReg();
+  uint32_t extraValuesToPop = IsConstructorCallOp(op) ? 3 : 2;
+  Register spReg = AsRegister(masm.getStackPointer());
+  masm.loadPtr(frame.addressOfInterpreterPC(), scratch);
+  LoadUint16Operand(masm, scratch, scratch);
+  masm.computeEffectiveAddress(
+      BaseValueIndex(spReg, scratch, extraValuesToPop * sizeof(Value)), spReg);
+  frame.push(R0);
+  return true;
 }
 
 template <typename Handler>
 bool BaselineCodeGen<Handler>::emitSpreadCall(JSOp op) {
   MOZ_ASSERT(IsCallOp(op));
 
   frame.syncStack(0);
   masm.move32(Imm32(1), R0.scratchReg());
--- a/js/src/jit/BaselineCompiler.h
+++ b/js/src/jit/BaselineCompiler.h
@@ -623,24 +623,27 @@ class BaselineCompiler final : private B
   MOZ_MUST_USE bool emitDebugTrap();
 
   MOZ_MUST_USE bool addPCMappingEntry(bool addIndexEntry);
 };
 
 // Interface used by BaselineCodeGen for BaselineInterpreterGenerator.
 class BaselineInterpreterHandler {
   InterpreterFrameInfo frame_;
+  Label interpretOp_;
 
  public:
   using FrameInfoT = InterpreterFrameInfo;
 
   explicit BaselineInterpreterHandler(JSContext* cx, MacroAssembler& masm);
 
   InterpreterFrameInfo& frame() { return frame_; }
 
+  Label* interpretOpLabel() { return &interpretOp_; }
+
   // Interpreter doesn't know the script and pc statically.
   jsbytecode* maybePC() const { return nullptr; }
   bool isDefinitelyLastOp() const { return false; }
   JSScript* maybeScript() const { return nullptr; }
   JSFunction* maybeFunction() const { return nullptr; }
 
   // Interpreter doesn't need to keep track of RetAddrEntries, so these methods
   // are no-ops.
--- a/js/src/jit/MacroAssembler.h
+++ b/js/src/jit/MacroAssembler.h
@@ -956,16 +956,17 @@ class MacroAssembler : public MacroAssem
 
   inline void divFloat32(FloatRegister src, FloatRegister dest) PER_SHARED_ARCH;
   inline void divDouble(FloatRegister src, FloatRegister dest) PER_SHARED_ARCH;
 
   inline void inc64(AbsoluteAddress dest) PER_ARCH;
 
   inline void neg32(Register reg) PER_SHARED_ARCH;
   inline void neg64(Register64 reg) DEFINED_ON(x86, x64, arm, mips32, mips64);
+  inline void negPtr(Register reg) PER_ARCH;
 
   inline void negateFloat(FloatRegister reg) PER_SHARED_ARCH;
 
   inline void negateDouble(FloatRegister reg) PER_SHARED_ARCH;
 
   inline void absFloat32(FloatRegister src, FloatRegister dest) PER_SHARED_ARCH;
   inline void absDouble(FloatRegister src, FloatRegister dest) PER_SHARED_ARCH;
 
--- a/js/src/jit/arm/MacroAssembler-arm-inl.h
+++ b/js/src/jit/arm/MacroAssembler-arm-inl.h
@@ -493,16 +493,18 @@ void MacroAssembler::inc64(AbsoluteAddre
 
 void MacroAssembler::neg32(Register reg) { ma_neg(reg, reg, SetCC); }
 
 void MacroAssembler::neg64(Register64 reg) {
   as_rsb(reg.low, reg.low, Imm8(0), SetCC);
   as_rsc(reg.high, reg.high, Imm8(0));
 }
 
+void MacroAssembler::negPtr(Register reg) { neg32(reg); }
+
 void MacroAssembler::negateDouble(FloatRegister reg) { ma_vneg(reg, reg); }
 
 void MacroAssembler::negateFloat(FloatRegister reg) { ma_vneg_f32(reg, reg); }
 
 void MacroAssembler::absFloat32(FloatRegister src, FloatRegister dest) {
   if (src != dest) {
     ma_vmov_f32(src, dest);
   }
--- a/js/src/jit/arm64/MacroAssembler-arm64-inl.h
+++ b/js/src/jit/arm64/MacroAssembler-arm64-inl.h
@@ -478,16 +478,20 @@ void MacroAssembler::inc64(AbsoluteAddre
   Add(scratch64, scratch64, Operand(1));
   Str(scratch64, MemOperand(scratchAddr64, 0));
 }
 
 void MacroAssembler::neg32(Register reg) {
   Negs(ARMRegister(reg, 32), Operand(ARMRegister(reg, 32)));
 }
 
+void MacroAssembler::negPtr(Register reg) {
+  Negs(ARMRegister(reg, 64), Operand(ARMRegister(reg, 64)));
+}
+
 void MacroAssembler::negateFloat(FloatRegister reg) {
   fneg(ARMFPRegister(reg, 32), ARMFPRegister(reg, 32));
 }
 
 void MacroAssembler::negateDouble(FloatRegister reg) {
   fneg(ARMFPRegister(reg, 64), ARMFPRegister(reg, 64));
 }
 
--- a/js/src/jit/x64/MacroAssembler-x64-inl.h
+++ b/js/src/jit/x64/MacroAssembler-x64-inl.h
@@ -259,16 +259,18 @@ void MacroAssembler::inc64(AbsoluteAddre
     ScratchRegisterScope scratch(*this);
     mov(ImmPtr(dest.addr), scratch);
     addPtr(Imm32(1), Address(scratch, 0));
   }
 }
 
 void MacroAssembler::neg64(Register64 reg) { negq(reg.reg); }
 
+void MacroAssembler::negPtr(Register reg) { negq(reg); }
+
 // ===============================================================
 // Shift functions
 
 void MacroAssembler::lshiftPtr(Imm32 imm, Register dest) {
   MOZ_ASSERT(0 <= imm.value && imm.value < 64);
   shlq(imm, dest);
 }
 
--- a/js/src/jit/x86/MacroAssembler-x86-inl.h
+++ b/js/src/jit/x86/MacroAssembler-x86-inl.h
@@ -338,16 +338,18 @@ void MacroAssembler::inc64(AbsoluteAddre
 }
 
 void MacroAssembler::neg64(Register64 reg) {
   negl(reg.low);
   adcl(Imm32(0), reg.high);
   negl(reg.high);
 }
 
+void MacroAssembler::negPtr(Register reg) { negl(reg); }
+
 // ===============================================================
 // Shift functions
 
 void MacroAssembler::lshiftPtr(Imm32 imm, Register dest) {
   MOZ_ASSERT(0 <= imm.value && imm.value < 32);
   shll(imm, dest);
 }
 
--- a/js/src/vm/BytecodeUtil.h
+++ b/js/src/vm/BytecodeUtil.h
@@ -587,21 +587,23 @@ inline bool IsCallOp(JSOp op) { return C
 
 inline bool IsCallPC(jsbytecode* pc) { return IsCallOp(JSOp(*pc)); }
 
 inline bool IsStrictEvalPC(jsbytecode* pc) {
   JSOp op = JSOp(*pc);
   return op == JSOP_STRICTEVAL || op == JSOP_STRICTSPREADEVAL;
 }
 
-inline bool IsConstructorCallPC(jsbytecode* pc) {
-  JSOp op = JSOp(*pc);
+inline bool IsConstructorCallOp(JSOp op) {
   return op == JSOP_NEW || op == JSOP_SUPERCALL || op == JSOP_SPREADNEW ||
          op == JSOP_SPREADSUPERCALL;
 }
+inline bool IsConstructorCallPC(const jsbytecode* pc) {
+  return IsConstructorCallOp(JSOp(*pc));
+}
 
 inline bool IsSpreadCallPC(jsbytecode* pc) {
   JSOp op = JSOp(*pc);
   return op == JSOP_SPREADCALL || op == JSOP_SPREADNEW ||
          op == JSOP_SPREADSUPERCALL || op == JSOP_SPREADEVAL ||
          op == JSOP_STRICTSPREADEVAL;
 }
 
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -1974,16 +1974,17 @@
      *   Category: Statements
      *   Type: Function
      *   Operands: uint8_t prefixKind
      *   Stack: fun, name => fun
      */ \
     MACRO(JSOP_SETFUNNAME, 182, "setfunname", NULL, 2, 2, 1, JOF_UINT8) \
     /*
      * Moves the top of the stack value under the nth element of the stack.
+     * Note: n must NOT be 0.
      *
      *   Category: Operators
      *   Type: Stack Operations
      *   Operands: uint8_t n
      *   Stack: v[n], v[n-1], ..., v[1], v[0] => v[0], v[n], v[n-1], ..., v[1]
      */ \
     MACRO(JSOP_UNPICK, 183, "unpick", NULL, 2, 0, 0, JOF_UINT8) \
     /*