Bug 584279 - TM: improve alias analysis by adding many more access regions (2nd attempt). r=bhackett.

js/src/jsarray.cpp

@@ -433,6 +433,9 @@ SetArrayElement(JSContext *cx, JSObject *obj, jsdouble index, const Value &v)
 JSBool JS_FASTCALL
 js_EnsureDenseArrayCapacity(JSContext *cx, JSObject *obj, jsint i)
 {
+#ifdef DEBUG
+    Class *origObjClasp = obj->clasp; 
+#endif
     jsuint u = jsuint(i);
     jsuint capacity = obj->getDenseArrayCapacity();
     if (u < capacity)
@@ -440,10 +443,15 @@ js_EnsureDenseArrayCapacity(JSContext *cx, JSObject *obj, jsint i)
     if (INDEX_TOO_SPARSE(obj, u))
         return false;
 
-    return obj->ensureDenseArrayElements(cx, u + 1);
+    JSBool ret = obj->ensureDenseArrayElements(cx, u + 1);
+
+    /* Partially check the CallInfo's storeAccSet is correct. */
+    JS_ASSERT(obj->clasp == origObjClasp);
+    return ret;
 }
-JS_DEFINE_CALLINFO_3(extern, BOOL, js_EnsureDenseArrayCapacity, CONTEXT, OBJECT, INT32, 0,
+/* This function and its callees do not touch any object's .clasp field. */
-                     nanojit::ACCSET_STORE_ANY)
+JS_DEFINE_CALLINFO_3(extern, BOOL, js_EnsureDenseArrayCapacity, CONTEXT, OBJECT, INT32,
+                     0, nanojit::ACCSET_STORE_ANY & ~ACCSET_OBJ_CLASP)
 #endif
 
 static JSBool
@@ -819,8 +827,9 @@ js_Array_dense_setelem_hole(JSContext* cx, JSObject* obj, jsint i)
         obj->setArrayLength(u + 1);
     return true;
 }
+/* storeAccSet == ACCSET_OBJ_PRIVATE: because it can set 'length'. */
 JS_DEFINE_CALLINFO_3(extern, BOOL, js_Array_dense_setelem_hole, CONTEXT, OBJECT, INT32,
-                     0, nanojit::ACCSET_STORE_ANY)
+                     0, ACCSET_OBJ_PRIVATE)
 #endif
 
 static JSBool

js/src/jsbuiltins.h

@@ -49,6 +49,95 @@
 #undef THIS
 #endif
 
+namespace js {
+
+/*
+ * See ValidateWriter::checkAccSet() for what each of these access regions
+ * mean.
+ *
+ * *** WARNING WARNING WARNING ***
+ *
+ * Any incorrect access region annotations on loads/stores/calls could lead to
+ * subtle bugs that manifest rarely, eg. when two loads are CSE'd that
+ * shouldn't be.
+ *
+ * If you add a new access region you will need to add some sanity checking to
+ * ValidateWriter::checkAccSet().  Do not skimp on this checking!  Make it as
+ * strong as you can.  Look at the existing cases for inspiration.  This
+ * checking helps prevent these subtle bugs.
+ *
+ * Furthermore, do not add a "catch-all" region such as "ACCSET_OTHER".  There
+ * are two reasons for this.  First, no checking could be done on loads/stores
+ * bearing it.  Second, it would be too easy for someone in the future who
+ * doesn't understand how AccSets work to use it inappropriately.  Only
+ * ACCSET_ALL (the union of all access regions) should be used as a catch-all,
+ * it can always be used safely, but it reduces optimization possibilities.
+ *
+ * Most of the access regions are type-based, ie. all structs of a particular
+ * type combined together form a region.  This is less precise than
+ * considering each struct separately, but also much simpler.
+ *
+ * - ACCSET_STATE:         The TracerState struct.
+ * - ACCSET_STACK:         The stack.
+ * - ACCSET_RSTACK:        The return stack.
+ * - ACCSET_CX:            All JSContext structs.
+ * - ACCSET_EOS:           The globals area.
+ * - ACCSET_ALLOC:         All memory blocks allocated with LIR_allocp (in
+ *                         other words, this region is the AR space).
+ * - ACCSET_FRAMEREGS:     All JSFrameRegs structs.
+ * - ACCSET_STACKFRAME:    All JSStackFrame objects.
+ * - ACCSET_RUNTIME:       The JSRuntime object.
+ * - ACCSET_OBJ_CLASP:     The 'clasp'    field of all JSObjects.
+ * - ACCSET_OBJ_SHAPE:     The 'shape'    field of all JSObjects.
+ * - ACCSET_OBJ_PROTO:     The 'proto'    field of all JSObjects.
+ * - ACCSET_OBJ_PARENT:    The 'parent'   field of all JSObjects.
+ * - ACCSET_OBJ_PRIVATE:   The 'private'  field of all JSObjects.
+ * - ACCSET_OBJ_CAPACITY:  The 'capacity' field of all JSObjects.
+ * - ACCSET_OBJ_SLOTS:     The 'slots'    field of all JSObjects.
+ * - ACCSET_SLOTS:         The slots (be they fixed or dynamic) of all JSObjects.
+ * - ACCSET_TARRAY:        All TypedArray structs.
+ * - ACCSET_TARRAY_DATA:   All TypedArray data arrays.
+ * - ACCSET_ITER:          All NativeIterator structs.
+ * - ACCSET_ITER_PROPS:    The props_arrays of all NativeIterator structs.
+ * - ACCSET_STRING:        All JSString structs.
+ * - ACCSET_STRING_MCHARS: All JSString mchars arrays.
+ * - ACCSET_TYPEMAP:       All typemaps form a single region.
+ * - ACCSET_FCSLOTS:       All fcslots arrays form a single region.
+ * - ACCSET_ARGS_DATA:     All Arguments data arrays form a single region.
+ */
+static const nanojit::AccSet ACCSET_STATE         = (1 <<  0);
+static const nanojit::AccSet ACCSET_STACK         = (1 <<  1);
+static const nanojit::AccSet ACCSET_RSTACK        = (1 <<  2);
+static const nanojit::AccSet ACCSET_CX            = (1 <<  3);
+static const nanojit::AccSet ACCSET_EOS           = (1 <<  4);
+static const nanojit::AccSet ACCSET_ALLOC         = (1 <<  5);
+static const nanojit::AccSet ACCSET_FRAMEREGS     = (1 <<  6);
+static const nanojit::AccSet ACCSET_STACKFRAME    = (1 <<  7);
+static const nanojit::AccSet ACCSET_RUNTIME       = (1 <<  8);
+
+// Nb: JSObject::{lastProp,map,flags} don't have an AccSet because they are never accessed on trace
+static const nanojit::AccSet ACCSET_OBJ_CLASP     = (1 <<  9);
+static const nanojit::AccSet ACCSET_OBJ_SHAPE     = (1 << 10);
+static const nanojit::AccSet ACCSET_OBJ_PROTO     = (1 << 11);
+static const nanojit::AccSet ACCSET_OBJ_PARENT    = (1 << 12);
+static const nanojit::AccSet ACCSET_OBJ_PRIVATE   = (1 << 13);
+static const nanojit::AccSet ACCSET_OBJ_CAPACITY  = (1 << 14);
+static const nanojit::AccSet ACCSET_OBJ_SLOTS     = (1 << 15);  // the pointer to the slots
+
+static const nanojit::AccSet ACCSET_SLOTS         = (1 << 16);  // the slots themselves
+static const nanojit::AccSet ACCSET_TARRAY        = (1 << 17);
+static const nanojit::AccSet ACCSET_TARRAY_DATA   = (1 << 18);
+static const nanojit::AccSet ACCSET_ITER          = (1 << 19);
+static const nanojit::AccSet ACCSET_ITER_PROPS    = (1 << 20);
+static const nanojit::AccSet ACCSET_STRING        = (1 << 21);
+static const nanojit::AccSet ACCSET_STRING_MCHARS = (1 << 22);
+static const nanojit::AccSet ACCSET_TYPEMAP       = (1 << 23);
+static const nanojit::AccSet ACCSET_FCSLOTS       = (1 << 24);
+static const nanojit::AccSet ACCSET_ARGS_DATA     = (1 << 25);
+}
+
+static const uint8_t TM_NUM_USED_ACCS = 26; // number of access regions used by TraceMonkey
+
 enum JSTNErrType { INFALLIBLE, FAIL_STATUS, FAIL_NULL, FAIL_NEG, FAIL_NEITHER };
 enum { 
     JSTN_ERRTYPE_MASK        = 0x07,

js/src/jsobj.h

@@ -239,6 +239,10 @@ struct NativeIterator;
 
 struct JSFunction;
 
+namespace nanojit {
+class ValidateWriter;
+}
+
 /*
  * JSObject struct, with members sized to fit in 32 bytes on 32-bit targets,
  * 64 bytes on 64-bit systems. The JSFunction struct is an extension of this
@@ -271,13 +275,19 @@ struct JSFunction;
  * hasSlotsArray().  In all cases, capacity gives the number of usable slots.
  * Two objects with the same shape have the same number of fixed slots,
  * and either both have or neither have dynamically allocated slot arrays.
+ *
+ * If you change this struct, you'll probably need to change the AccSet values
+ * in jsbuiltins.h.
  */
 struct JSObject : js::gc::Cell {
     /*
      * TraceRecorder must be a friend because it generates code that
      * manipulates JSObjects, which requires peeking under any encapsulation.
+     * ValidateWriter must be a friend because it works in tandem with
+     * TraceRecorder.
      */
     friend class js::TraceRecorder;
+    friend class nanojit::ValidateWriter;
 
     /*
      * Private pointer to the last added property and methods to manipulate the

js/src/jstracer.h

@@ -55,22 +55,6 @@
 
 namespace js {
 
-/*
- * TM-specific access regions:
- *
- * - STACK: the stack.  STACK loads/stores always use 'sp' or 'sp+k' as the
- *   base pointer.
- *
- * - RSTACK: the return stack.  RSTACK loads/stores always use 'rp' as the
- *   base pointer.
- *
- * - OTHER: all other regions of memory.
- */
-static const nanojit::AccSet ACCSET_STACK   = (1 << 0);
-static const nanojit::AccSet ACCSET_RSTACK  = (1 << 1);
-static const nanojit::AccSet ACCSET_OTHER   = (1 << 2);
-static const uint8_t TM_NUM_USED_ACCS = 3;  // number of access regions used by TraceMonkey
-
 #if defined(DEBUG) && !defined(JS_JIT_SPEW)
 #define JS_JIT_SPEW
 #endif
@@ -881,6 +865,7 @@ class TraceRecorder
 
     /* The LIR-generation pipeline used to build |fragment|. */
     nanojit::LirWriter* const       lir;
+    nanojit::CseFilter* const       cse_filter;
 
     /* Instructions yielding the corresponding trace-const members of TracerState. */
     nanojit::LIns* const            cx_ins;
@@ -916,8 +901,8 @@ class TraceRecorder
     /* Maps interpreter stack values to the instruction writing back to the native stack. */
     Tracker                         nativeFrameTracker;
 
-    /* The start of the global object's dslots we assume for the trackers. */
+    /* The start of the global object's slots we assume for the trackers. */
-    Value*                          global_dslots;
+    Value*                          global_slots;
 
     /* The number of interpreted calls entered (and not yet left) since recording began. */
     unsigned                        callDepth;
@@ -972,6 +957,16 @@ class TraceRecorder
 
     /************************************************************* 10 bajillion member functions */
 
+    /* 
+     * These can be put around a control-flow diamond if it's important that
+     * CSE work across the diamond.  Duplicated expressions within the diamond
+     * will be CSE'd, but expressions defined within the diamond won't be
+     * added to the tables of CSEable expressions.  Loads are still
+     * invalidated if they alias any stores that occur within diamonds.
+     */
+    void suspendCSE()   { if (cse_filter) cse_filter->suspend(); }
+    void resumeCSE()    { if (cse_filter) cse_filter->resume();  }
+
     nanojit::LIns* insImmVal(const Value& val);
     nanojit::LIns* insImmObj(JSObject* obj);
     nanojit::LIns* insImmFun(JSFunction* fun);
@@ -1020,9 +1015,11 @@ class TraceRecorder
     JS_REQUIRES_STACK ptrdiff_t nativeStackSlot(const Value* p) const;
     JS_REQUIRES_STACK ptrdiff_t nativespOffsetImpl(const void* p) const;
     JS_REQUIRES_STACK ptrdiff_t nativespOffset(const Value* p) const;
-    JS_REQUIRES_STACK void importImpl(nanojit::LIns* base, ptrdiff_t offset, const void* p, JSValueType t,
+    JS_REQUIRES_STACK void importImpl(nanojit::LIns* base, ptrdiff_t offset, nanojit::AccSet accSet,
+                                      const void* p, JSValueType t,
                                       const char *prefix, uintN index, JSStackFrame *fp);
-    JS_REQUIRES_STACK void import(nanojit::LIns* base, ptrdiff_t offset, const Value* p, JSValueType t,
+    JS_REQUIRES_STACK void import(nanojit::LIns* base, ptrdiff_t offset, nanojit::AccSet accSet,
+                                  const Value* p, JSValueType t,
                                   const char *prefix, uintN index, JSStackFrame *fp);
     JS_REQUIRES_STACK void import(TreeFragment* tree, nanojit::LIns* sp, unsigned stackSlots,
                                   unsigned callDepth, unsigned ngslots, JSValueType* typeMap);
@@ -1065,12 +1062,12 @@ class TraceRecorder
     JS_REQUIRES_STACK bool known(const Value* p);
     JS_REQUIRES_STACK bool known(JSObject** p);
     /*
-     * The dslots of the global object are sometimes reallocated by the
+     * The slots of the global object are sometimes reallocated by the
      * interpreter.  This function checks for that condition and re-maps the
      * entries of the tracker accordingly.
      */
     JS_REQUIRES_STACK void checkForGlobalObjectReallocation() {
-        if (global_dslots != globalObj->getSlots())
+        if (global_slots != globalObj->getSlots())
             checkForGlobalObjectReallocationHelper();
     }
     JS_REQUIRES_STACK void checkForGlobalObjectReallocationHelper();
@@ -1169,6 +1166,7 @@ class TraceRecorder
     void forgetGuardedShapes();
 
     inline nanojit::LIns* shape_ins(nanojit::LIns *obj_ins);
+    inline nanojit::LIns* slots(nanojit::LIns *obj_ins);
     JS_REQUIRES_STACK AbortableRecordingStatus test_property_cache(JSObject* obj, nanojit::LIns* obj_ins,
                                                                      JSObject*& obj2, PCVal& pcval);
     JS_REQUIRES_STACK RecordingStatus guardPropertyCacheHit(nanojit::LIns* obj_ins,
@@ -1180,9 +1178,9 @@ class TraceRecorder
     void stobj_set_fslot(nanojit::LIns *obj_ins, unsigned slot, const Value &v,
                          nanojit::LIns* v_ins);
     void stobj_set_dslot(nanojit::LIns *obj_ins, unsigned slot,
-                         nanojit::LIns*& dslots_ins, const Value &v, nanojit::LIns* v_ins);
+                         nanojit::LIns*& slots_ins, const Value &v, nanojit::LIns* v_ins);
     void stobj_set_slot(JSObject *obj, nanojit::LIns* obj_ins, unsigned slot,
-                        nanojit::LIns*& dslots_ins, const Value &v, nanojit::LIns* v_ins);
+                        nanojit::LIns*& slots_ins, const Value &v, nanojit::LIns* v_ins);
 
     nanojit::LIns* stobj_get_slot_uint32(nanojit::LIns* obj_ins, unsigned slot);
     nanojit::LIns* unbox_slot(JSObject *obj, nanojit::LIns *obj_ins, uint32 slot,
@@ -1237,6 +1235,7 @@ class TraceRecorder
                                                                   nanojit::LIns* obj_ins,
                                                                   const js::Shape* shape);
 
+    JS_REQUIRES_STACK nanojit::LIns* getStringLengthAndFlags(nanojit::LIns* str_ins);
     JS_REQUIRES_STACK nanojit::LIns* getStringLength(nanojit::LIns* str_ins);
     JS_REQUIRES_STACK nanojit::LIns* getStringChars(nanojit::LIns* str_ins);
     JS_REQUIRES_STACK RecordingStatus getCharCodeAt(JSString *str,
@@ -1287,7 +1286,7 @@ class TraceRecorder
 #endif
 
     nanojit::LIns* unbox_value(const Value& v, nanojit::LIns* vaddr_ins,
-                               ptrdiff_t offset, VMSideExit* exit,
+                               ptrdiff_t offset, nanojit::AccSet accSet, VMSideExit* exit,
                                bool force_double=false);
     void unbox_any_object(nanojit::LIns* vaddr_ins, nanojit::LIns** obj_ins,
                           nanojit::LIns** is_obj_ins, nanojit::AccSet accSet);