Bug 1537908 part 2 - Store numICEntries in SharedScriptData. r=tcampbell
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 26 Apr 2019 09:16:49 +0000
changeset 530282 39133aa95fce7f894c55ace40c778983c8ca26bf
parent 530281 f35cf62b01a185a3cf8120ac340a347bb23dccf0
child 530283 50967daabdf4ed31cc819b40e6dad09b62f2dccf
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1537908
milestone68.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 1537908 part 2 - Store numICEntries in SharedScriptData. r=tcampbell This also tidies up overflow checks to ensure numICEntries never overflows UINT32_MAX in the emitter. ICScript currently stores numICEntries as uint32_t, but it's not an issue due to BaselineMaxScriptLength. Differential Revision: https://phabricator.services.mozilla.com/D28450
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/frontend/LabelEmitter.cpp
js/src/frontend/LabelEmitter.h
js/src/vm/JSScript.cpp
js/src/vm/JSScript.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -239,16 +239,23 @@ bool BytecodeEmitter::emitCheck(JSOp op,
 
   // If op is JOF_TYPESET (see the type barriers comment in TypeInference.h),
   // reserve a type set to store its result.
   if (CodeSpec[op].format & JOF_TYPESET) {
     bytecodeSection().incrementNumTypeSets();
   }
 
   if (BytecodeOpHasIC(op)) {
+    // Because numICEntries also includes entries for formal arguments, we have
+    // to check for overflow here.
+    if (MOZ_UNLIKELY(bytecodeSection().numICEntries() == UINT32_MAX)) {
+      reportError(nullptr, JSMSG_NEED_DIET, js_script_str);
+      return false;
+    }
+
     bytecodeSection().incrementNumICEntries();
   }
 
   return true;
 }
 
 void BytecodeEmitter::BytecodeSection::updateDepth(ptrdiff_t target) {
   jsbytecode* pc = code(target);
@@ -351,21 +358,17 @@ bool BytecodeEmitter::emitN(JSOp op, siz
     *offset = off;
   }
   return true;
 }
 
 bool BytecodeEmitter::emitJumpTargetOp(JSOp op, ptrdiff_t* off) {
   MOZ_ASSERT(BytecodeIsJumpTarget(op));
 
-  size_t numEntries = bytecodeSection().numICEntries();
-  if (MOZ_UNLIKELY(numEntries > UINT32_MAX)) {
-    reportError(nullptr, JSMSG_NEED_DIET, js_script_str);
-    return false;
-  }
+  uint32_t numEntries = bytecodeSection().numICEntries();
 
   if (!emitN(op, CodeSpec[op].length - 1, off)) {
     return false;
   }
 
   SET_ICINDEX(bytecodeSection().code(*off), numEntries);
   return true;
 }
@@ -1766,17 +1769,17 @@ bool BytecodeEmitter::emitGetNameAtLocat
 
   return true;
 }
 
 bool BytecodeEmitter::emitGetName(NameNode* name) {
   return emitGetName(name->name());
 }
 
-bool BytecodeEmitter::emitTDZCheckIfNeeded(JSAtom* name,
+bool BytecodeEmitter::emitTDZCheckIfNeeded(HandleAtom name,
                                            const NameLocation& loc) {
   // Dynamic accesses have TDZ checks built into their VM code and should
   // never emit explicit TDZ checks.
   MOZ_ASSERT(loc.hasKnownSlot());
   MOZ_ASSERT(loc.isLexical());
 
   Maybe<MaybeCheckTDZ> check =
       innermostTDZCheckCache->needsTDZCheck(this, name);
@@ -7600,17 +7603,18 @@ MOZ_NEVER_INLINE bool BytecodeEmitter::e
   }
 }
 
 // Using MOZ_NEVER_INLINE in here is a workaround for llvm.org/pr14047. See
 // the comment on emitSwitch.
 MOZ_NEVER_INLINE bool BytecodeEmitter::emitLabeledStatement(
     const LabeledStatement* labeledStmt) {
   LabelEmitter label(this);
-  if (!label.emitLabel(labeledStmt->label())) {
+  RootedAtom name(cx, labeledStmt->label());
+  if (!label.emitLabel(name)) {
     return false;
   }
   if (!emitTree(labeledStmt->statement())) {
     return false;
   }
   if (!label.emitEnd()) {
     return false;
   }
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -218,22 +218,28 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
 
     bool isDuplicateLocation() const {
       return lastSeparatorLine_ == currentLine_ &&
              lastSeparatorColumn_ == lastColumn_;
     }
 
     // ---- JIT ----
 
-    size_t numICEntries() const { return numICEntries_; }
-    void incrementNumICEntries() { numICEntries_++; }
-    void setNumICEntries(size_t entries) { numICEntries_ = entries; }
+    uint32_t numICEntries() const { return numICEntries_; }
+    void incrementNumICEntries() {
+      MOZ_ASSERT(numICEntries_ != UINT32_MAX, "Shouldn't overflow");
+      numICEntries_++;
+    }
+    void setNumICEntries(uint32_t entries) { numICEntries_ = entries; }
 
     uint32_t numTypeSets() const { return numTypeSets_; }
-    void incrementNumTypeSets() { numTypeSets_++; }
+    void incrementNumTypeSets() {
+      MOZ_ASSERT(numTypeSets_ != UINT32_MAX, "Shouldn't overflow");
+      numTypeSets_++;
+    }
 
    private:
     // ---- Bytecode ----
 
     // Bytecode.
     BytecodeVector code_;
 
     // ---- Source notes ----
@@ -297,18 +303,19 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
     // The offset, line and column numbers of the last opcode for the
     // breakpoint for step execution.
     uint32_t lastSeparatorOffet_ = 0;
     uint32_t lastSeparatorLine_ = 0;
     uint32_t lastSeparatorColumn_ = 0;
 
     // ---- JIT ----
 
-    // Number of JOF_IC opcodes emitted.
-    size_t numICEntries_ = 0;
+    // Number of ICEntries in the script. There's one ICEntry for each JOF_IC op
+    // and, if the script is a function, for |this| and each formal argument.
+    uint32_t numICEntries_ = 0;
 
     // Number of JOF_TYPESET opcodes generated.
     uint32_t numTypeSets_ = 0;
   };
 
   BytecodeSection bytecodeSection_;
 
  public:
@@ -795,17 +802,18 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
 
   MOZ_MUST_USE bool emitGetNameAtLocation(JSAtom* name,
                                           const NameLocation& loc);
   MOZ_MUST_USE bool emitGetName(JSAtom* name) {
     return emitGetNameAtLocation(name, lookupName(name));
   }
   MOZ_MUST_USE bool emitGetName(NameNode* name);
 
-  MOZ_MUST_USE bool emitTDZCheckIfNeeded(JSAtom* name, const NameLocation& loc);
+  MOZ_MUST_USE bool emitTDZCheckIfNeeded(HandleAtom name,
+                                         const NameLocation& loc);
 
   MOZ_MUST_USE bool emitNameIncDec(UnaryNode* incDec);
 
   MOZ_MUST_USE bool emitDeclarationList(ListNode* declList);
   MOZ_MUST_USE bool emitSingleDeclaration(ListNode* declList, NameNode* decl,
                                           ParseNode* initializer);
 
   MOZ_MUST_USE bool emitNewInit();
--- a/js/src/frontend/LabelEmitter.cpp
+++ b/js/src/frontend/LabelEmitter.cpp
@@ -9,17 +9,17 @@
 #include "mozilla/Assertions.h"  // MOZ_ASSERT
 
 #include "frontend/BytecodeEmitter.h"  // BytecodeEmitter
 #include "vm/Opcodes.h"                // JSOP_*
 
 using namespace js;
 using namespace js::frontend;
 
-bool LabelEmitter::emitLabel(JSAtom* name) {
+bool LabelEmitter::emitLabel(HandleAtom name) {
   MOZ_ASSERT(state_ == State::Start);
 
   // Emit a JSOP_LABEL instruction. The operand is the offset to the statement
   // following the labeled statement. The offset is set in emitEnd().
   uint32_t index;
   if (!bce_->makeAtomIndex(name, &index)) {
     return false;
   }
--- a/js/src/frontend/LabelEmitter.h
+++ b/js/src/frontend/LabelEmitter.h
@@ -55,16 +55,16 @@ class MOZ_STACK_CLASS LabelEmitter {
     End
   };
   State state_ = State::Start;
 #endif
 
  public:
   explicit LabelEmitter(BytecodeEmitter* bce) : bce_(bce) {}
 
-  MOZ_MUST_USE bool emitLabel(JSAtom* name);
+  MOZ_MUST_USE bool emitLabel(HandleAtom name);
   MOZ_MUST_USE bool emitEnd();
 };
 
 } /* namespace frontend */
 } /* namespace js */
 
 #endif /* frontend_LabelEmitter_h */
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -770,16 +770,17 @@ XDRResult SharedScriptData::XDR(XDRState
     }
     ssd = script->scriptData();
   }
 
   MOZ_TRY(xdr->codeUint32(&ssd->mainOffset));
   MOZ_TRY(xdr->codeUint32(&ssd->nfixed));
   MOZ_TRY(xdr->codeUint32(&ssd->nslots));
   MOZ_TRY(xdr->codeUint32(&ssd->bodyScopeIndex));
+  MOZ_TRY(xdr->codeUint32(&ssd->numICEntries));
   MOZ_TRY(xdr->codeUint16(&ssd->funLength));
   MOZ_TRY(xdr->codeUint16(&ssd->numBytecodeTypeSets));
 
   JS_STATIC_ASSERT(sizeof(jsbytecode) == 1);
   JS_STATIC_ASSERT(sizeof(jssrcnote) == 1);
 
   jsbytecode* code = ssd->code();
   jssrcnote* notes = ssd->notes();
@@ -4520,16 +4521,17 @@ bool JSScript::hasBreakpointsAt(jsbyteco
 
   js::SharedScriptData* data = script->scriptData_;
 
   // Initialize POD fields
   data->mainOffset = bce->mainOffset();
   data->nfixed = bce->maxFixedSlots;
   data->nslots = nslots;
   data->bodyScopeIndex = bce->bodyScopeIndex;
+  data->numICEntries = bce->bytecodeSection().numICEntries();
   data->numBytecodeTypeSets =
       std::min<uint32_t>(uint32_t(JSScript::MaxBytecodeTypeSets),
                          bce->bytecodeSection().numTypeSets());
 
   if (bce->sc->isFunctionBox()) {
     data->funLength = bce->sc->asFunctionBox()->length;
   }
 
--- a/js/src/vm/JSScript.h
+++ b/js/src/vm/JSScript.h
@@ -1515,19 +1515,18 @@ class alignas(uintptr_t) SharedScriptDat
   uint32_t nfixed = 0;
 
   // Slots plus maximum stack depth.
   uint32_t nslots = 0;
 
   // Index into the scopes array of the body scope.
   uint32_t bodyScopeIndex = 0;
 
-#if JS_BITS_PER_WORD == 64
-  uint32_t padding_ = 0;
-#endif
+  // Number of IC entries to allocate in ICScript for Baseline ICs.
+  uint32_t numICEntries = 0;
 
   // ES6 function length.
   uint16_t funLength = 0;
 
   // Number of type sets used in this script for dynamic type monitoring.
   uint16_t numBytecodeTypeSets = 0;
 
   // NOTE: The raw bytes of this structure are used for hashing so use explicit
@@ -2157,16 +2156,18 @@ class JSScript : public js::gc::TenuredC
   static constexpr size_t MaxBytecodeTypeSets = UINT16_MAX;
   static_assert(sizeof(js::SharedScriptData::numBytecodeTypeSets) == 2,
                 "MaxBytecodeTypeSets must match sizeof(numBytecodeTypeSets)");
 
   size_t numBytecodeTypeSets() const {
     return scriptData_->numBytecodeTypeSets;
   }
 
+  size_t numICEntries() const { return scriptData_->numICEntries; }
+
   size_t funLength() const { return scriptData_->funLength; }
 
   uint32_t sourceStart() const { return sourceStart_; }
 
   uint32_t sourceEnd() const { return sourceEnd_; }
 
   uint32_t sourceLength() const { return sourceEnd_ - sourceStart_; }