Bug 941715 - SpiderMonkey: Don't use DebugOnly in struct fields when …

…size is relevant. r=jorendorff
ayubamini · Dec 11, 2013 · e1b051e · e1b051e
1 parent bd1ec7e
commit e1b051e
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 14 deletions.
diff --git a/js/src/jit/AsmJSModule.cpp b/js/src/jit/AsmJSModule.cpp
@@ -10,7 +10,6 @@
 # include <sys/mman.h>
 #endif
 
-#include "mozilla/DebugOnly.h"
 #include "mozilla/PodOperations.h"
 
 #include "jslibmath.h"
@@ -32,7 +31,6 @@
 using namespace js;
 using namespace jit;
 using namespace frontend;
-using mozilla::DebugOnly;
 using mozilla::PodEqual;
 
 void

diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp
@@ -4234,7 +4234,9 @@ ICGetElemNativeCompiler::generateStubCode(MacroAssembler &masm)
     }
 
     // Since this stub sometimes enter a stub frame, we manually set this to true (lie).
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
 
     // Key has been atomized if necessary.  Do identity check on string pointer.
     masm.branchPtr(Assembler::NotEqual, nameAddr, strExtract, &failure);
@@ -4477,7 +4479,9 @@ ICGetElem_Dense::Compiler::generateStubCode(MacroAssembler &masm)
 
     // Check if __noSuchMethod__ should be called.
 #if JS_HAS_NO_SUCH_METHOD
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
     if (isCallElem_) {
         Label afterNoSuchMethod;
         Label skipNoSuchMethod;
@@ -4607,7 +4611,9 @@ ICGetElem_Arguments::Compiler::generateStubCode(MacroAssembler &masm)
     // Variatns of GetElem_Arguments can enter stub frames if entered in CallProp
     // context when noSuchMethod support is on.
 #if JS_HAS_NO_SUCH_METHOD
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
 #endif
 
     Label failure;
@@ -6378,7 +6384,9 @@ ICGetProp_Fallback::Compiler::generateStubCode(MacroAssembler &masm)
 
     // Even though the fallback frame doesn't enter a stub frame, the CallScripted
     // frame that we are emulating does. Again, we lie.
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
 
     leaveStubFrame(masm, true);
 
@@ -6562,7 +6570,9 @@ ICGetPropNativeCompiler::generateStubCode(MacroAssembler &masm)
     BaseIndex result(holderReg, scratch, TimesOne);
 
 #if JS_HAS_NO_SUCH_METHOD
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
     if (isCallProp_) {
         // Check for __noSuchMethod__ invocation.
         Label afterNoSuchMethod;
@@ -7298,7 +7308,9 @@ ICSetProp_Fallback::Compiler::generateStubCode(MacroAssembler &masm)
 
     // Even though the fallback frame doesn't enter a stub frame, the CallScripted
     // frame that we are emulating does. Again, we lie.
+#ifdef DEBUG
     entersStubFrame_ = true;
+#endif
 
     leaveStubFrame(masm, true);
 

diff --git a/js/src/jit/BaselineIC.h b/js/src/jit/BaselineIC.h
@@ -998,9 +998,11 @@ class ICStubCompiler
 
 
   protected:
-    mozilla::DebugOnly<bool> entersStubFrame_;
     JSContext *cx;
     ICStub::Kind kind;
+#ifdef DEBUG
+    bool entersStubFrame_;
+#endif
 
     // By default the stubcode key is just the kind.
     virtual int32_t getKey() const {
@@ -1014,7 +1016,10 @@ class ICStubCompiler
     IonCode *getStubCode();
 
     ICStubCompiler(JSContext *cx, ICStub::Kind kind)
-      : suppressGC(cx), entersStubFrame_(false), cx(cx), kind(kind)
+      : suppressGC(cx), cx(cx), kind(kind)
+#ifdef DEBUG
+      , entersStubFrame_(false)
+#endif
     {}
 
     // Emits a tail call to a VMFunction wrapper.

diff --git a/js/src/jit/InlineList.h b/js/src/jit/InlineList.h
@@ -7,8 +7,6 @@
 #ifndef jit_InlineList_h
 #define jit_InlineList_h
 
-#include "mozilla/DebugOnly.h"
-
 #include "jsutil.h"
 
 namespace js {
@@ -40,7 +38,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
     typedef InlineForwardListNode<T> Node;
 
     Node *tail_;
-    mozilla::DebugOnly<int> modifyCount_;
+#ifdef DEBUG
+    int modifyCount_;
+#endif
 
     InlineForwardList<T> *thisFromConstructor() {
         return this;
@@ -50,7 +50,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
     InlineForwardList()
       : tail_(thisFromConstructor())
     {
+#ifdef DEBUG
         modifyCount_ = 0;
+#endif
     }
 
   public:
@@ -67,7 +69,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
         iterator iter(where);
         iter++;
         iter.prev = where.prev;
+#ifdef DEBUG
         iter.modifyCount_++;
+#endif
 
         // Once the element 'where' points at has been removed, it is no longer
         // safe to do any operations that would touch 'iter', as the element
@@ -82,7 +86,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
         insertAfter(this, t);
     }
     void pushBack(Node *t) {
+#ifdef DEBUG
         modifyCount_++;
+#endif
         tail_->next = t;
         t->next = nullptr;
         tail_ = t;
@@ -98,14 +104,18 @@ class InlineForwardList : protected InlineForwardListNode<T>
         return static_cast<T *>(tail_);
     }
     void insertAfter(Node *at, Node *item) {
+#ifdef DEBUG
         modifyCount_++;
+#endif
         if (at == tail_)
             tail_ = item;
         item->next = at->next;
         at->next = item;
     }
     void removeAfter(Node *at, Node *item) {
+#ifdef DEBUG
         modifyCount_++;
+#endif
         if (item == tail_)
             tail_ = at;
         JS_ASSERT(at->next == item);
@@ -117,7 +127,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
             at = this;
         if (at == tail_)
             return;
+#ifdef DEBUG
         modifyCount_++;
+#endif
         to->next = at->next;
         to->tail_ = tail_;
         tail_ = at;
@@ -129,7 +141,9 @@ class InlineForwardList : protected InlineForwardListNode<T>
     void clear() {
         this->next = nullptr;
         tail_ = this;
+#ifdef DEBUG
         modifyCount_ = 0;
+#endif
     }
 };
 
@@ -146,7 +160,7 @@ class InlineForwardListIterator
         iter(owner ? owner->next : nullptr)
 #ifdef DEBUG
       , owner_(owner),
-        modifyCount_(owner ? owner->modifyCount_.value : 0)
+        modifyCount_(owner ? owner->modifyCount_ : 0)
 #endif
     { }
 
@@ -185,8 +199,8 @@ class InlineForwardListIterator
 
 #ifdef DEBUG
     const InlineForwardList<T> *owner_;
+    int modifyCount_;
 #endif
-    mozilla::DebugOnly<int> modifyCount_;
 };
 
 template <typename T> class InlineList;

diff --git a/js/src/jit/IonFrames-inl.h b/js/src/jit/IonFrames-inl.h
@@ -25,7 +25,9 @@ SafepointIndex::resolve()
 {
     JS_ASSERT(!resolved);
     safepointOffset_ = safepoint_->offset();
+#ifdef DEBUG
     resolved = true;
+#endif
 }
 
 inline uint8_t *

diff --git a/js/src/jit/IonFrames.h b/js/src/jit/IonFrames.h
@@ -9,8 +9,6 @@
 
 #ifdef JS_ION
 
-#include "mozilla/DebugOnly.h"
-
 #include "jscntxt.h"
 #include "jsfun.h"
 
@@ -103,13 +101,17 @@ class SafepointIndex
         uint32_t safepointOffset_;
     };
 
-    mozilla::DebugOnly<bool> resolved;
+#ifdef DEBUG
+    bool resolved;
+#endif
 
   public:
     SafepointIndex(uint32_t displacement, LSafepoint *safepoint)
       : displacement_(displacement),
-        safepoint_(safepoint),
-        resolved(false)
+        safepoint_(safepoint)
+#ifdef DEBUG
+      , resolved(false)
+#endif
     { }
 
     void resolve();

diff --git a/mfbt/DebugOnly.h b/mfbt/DebugOnly.h
@@ -27,6 +27,9 @@ namespace mozilla {
  *
  * DebugOnly instances can only be coerced to T in debug builds.  In release
  * builds they don't have a value, so type coercion is not well defined.
+ *
+ * Note that DebugOnly instances still take up one byte of space, plus padding,
+ * when used as members of structs.
  */
 template<typename T>
 class DebugOnly