[INFER] Don't use type barriers when compiling GETELEM on typed arrays, bug 678782.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 17 Aug 2011 16:11:38 -0700
changeset 76131 a4599ee114dcf8a03c788e9dc54bced248902d42
parent 76130 044290f5a31d7ee3216a5f884f827b11fb9faae2
child 76132 846a3a6322490693c5be2157cd26945423e02386
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
bugs678782
milestone8.0a1
[INFER] Don't use type barriers when compiling GETELEM on typed arrays, bug 678782.
js/src/jit-test/tests/jaeger/bug678782.js
js/src/jsarray.cpp
js/src/jsinfer.h
js/src/jsobjinlines.h
js/src/methodjit/Compiler.h
js/src/methodjit/FastOps.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/bug678782.js
@@ -0,0 +1,9 @@
+var i = -1; var j = -1; var s = ''; var f = '';
+var buf = serialize(new Date(NaN));
+var a = [1/0, -1/0, 8.64e15 + 1, -(8.64e15 + 1)];
+for (var i = 0; i < a.length; i++) {
+    var n = a[i];
+    var nbuf = serialize(n);
+    for (var Number ; j < 8; j++)
+        buf[j + 8] = nbuf[j];
+}
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -789,16 +789,21 @@ array_getProperty(JSContext *cx, JSObjec
             shape = (const Shape *) prop;
             if (!js_NativeGet(cx, obj, obj2, shape, JSGET_METHOD_BARRIER, vp))
                 return JS_FALSE;
         }
         return JS_TRUE;
     }
 
     *vp = obj->getDenseArrayElement(i);
+
+    /* Type information for dense array elements must be correct. */
+    JS_ASSERT_IF(!obj->hasSingletonType(),
+                 js::types::TypeHasProperty(cx, obj->type(), JSID_VOID, *vp));
+
     return JS_TRUE;
 }
 
 static JSBool
 slowarray_addProperty(JSContext *cx, JSObject *obj, jsid id, Value *vp)
 {
     jsuint index, length;
 
--- a/js/src/jsinfer.h
+++ b/js/src/jsinfer.h
@@ -559,16 +559,23 @@ struct TypeResult
  * where the result of the bytecode is used.
  *
  * Observing new types at a bytecode removes type barriers at the bytecode
  * (this removal happens lazily, see ScriptAnalysis::pruneTypeBarriers), and if
  * all type barriers at a bytecode are removed --- the set of observed types
  * grows to match the set of possible types --- then the result of the bytecode
  * no longer needs to be dynamically checked (unless the set of possible types
  * grows, triggering the generation of new type barriers).
+ *
+ * Barriers are only relevant for accesses on properties whose types inference
+ * actually tracks (see propertySet comment under TypeObject). Accesses on
+ * other properties may be able to produce additional unobserved types even
+ * without a barrier present, and can only be compiled to jitcode with special
+ * knowledge of the property in question (e.g. for lengths of arrays, or
+ * elements of typed arrays).
  */
 
 /*
  * Barrier introduced at some bytecode. These are added when, during inference,
  * we block a type from being propagated as would normally be done for a subset
  * constraint. The propagation is technically possible, but we suspect it will
  * not happen dynamically and this type needs to be watched for. These are only
  * added at reads of properties and at scripted call sites.
@@ -731,37 +738,36 @@ struct TypeObject : gc::Cell
      * is linear, not worst-case cubic as it would otherwise be.
      */
     uint32 contribution;
     static const uint32 CONTRIBUTION_LIMIT = 2000;
 
     /*
      * Properties of this object. This may contain JSID_VOID, representing the
      * types of all integer indexes of the object, and/or JSID_EMPTY, holding
-     * constraints listening to changes to the object's state. Correspondence
-     * between the properties of a TypeObject and the properties of
-     * script-visible JSObjects (not Call, Block, etc.) which have that type is
-     * as follows:
+     * constraints listening to changes to the object's state.
+     *
+     * The type sets in the properties of a type object describe the possible
+     * values that can be read out of that property in actual JS objects.
+     * Properties only account for native properties (those with a slot and no
+     * specialized getter hook) and the elements of dense arrays. For accesses
+     * on such properties, the correspondence is as follows:
      *
      * - If the type has unknownProperties(), the possible properties and value
      *   types for associated JSObjects are unknown.
      *
      * - Otherwise, for any JSObject obj with TypeObject type, and any jsid id
-     *   which is a property in obj, after obj->getProperty(id) the property in
+     *   which is a property in obj, before obj->getProperty(id) the property in
      *   type for id must reflect the result of the getProperty. The result is
      *   additionally allowed to be undefined for properties of global objects
      *   defined with 'var' but not yet written.
      *
-     * - Additionally, if id is a normal owned native property within obj, then
-     *   after the setProperty or defineProperty which wrote its value, the
-     *   property in type for id must reflect that type.
-     *
      * We establish these by using write barriers on calls to setProperty and
-     * defineProperty which are on native properties, and read barriers on
-     * getProperty that go through a class hook or special PropertyOp.
+     * defineProperty which are on native properties, and by using the inference
+     * analysis to determine the side effects of code which is JIT-compiled.
      */
     Property **propertySet;
 
     /* If this is an interpreted function, the corresponding script. */
     JSScript *functionScript;
 
     /* Make an object with the specified name. */
     inline TypeObject(jsid id, JSObject *proto, bool isFunction, bool unknown);
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -135,21 +135,21 @@ JSObject::setAttributes(JSContext *cx, j
 
 inline JSBool
 JSObject::getProperty(JSContext *cx, JSObject *receiver, jsid id, js::Value *vp)
 {
     js::PropertyIdOp op = getOps()->getProperty;
     if (op) {
         if (!op(cx, this, receiver, id, vp))
             return false;
-        js::types::AddTypePropertyId(cx, this, id, *vp);
     } else {
         if (!js_GetProperty(cx, this, receiver, id, vp))
             return false;
-        JS_ASSERT_IF(!hasSingletonType(), js::types::TypeHasProperty(cx, type(), id, *vp));
+        JS_ASSERT_IF(!hasSingletonType(),
+                     js::types::TypeHasProperty(cx, type(), id, *vp));
     }
     return true;
 }
 
 inline JSBool
 JSObject::getProperty(JSContext *cx, jsid id, js::Value *vp)
 {
     return getProperty(cx, this, id, vp);
--- a/js/src/methodjit/Compiler.h
+++ b/js/src/methodjit/Compiler.h
@@ -703,17 +703,17 @@ class Compiler : public BaseCompiler
     void jsop_setelem_typed(int atype);
     void convertForTypedArray(int atype, ValueRemat *vr, bool *allocated);
 #endif
     bool jsop_setelem(bool popGuaranteed);
     bool jsop_getelem(bool isCall);
     void jsop_getelem_dense(bool isPacked);
     void jsop_getelem_args();
 #ifdef JS_METHODJIT_TYPED_ARRAY
-    void jsop_getelem_typed(int atype);
+    bool jsop_getelem_typed(int atype);
 #endif
     void jsop_toid();
     bool isCacheableBaseAndIndex(FrameEntry *obj, FrameEntry *id);
     void jsop_stricteq(JSOp op);
     bool jsop_equality(JSOp op, BoolStub stub, jsbytecode *target, JSOp fused);
     bool jsop_equality_int_string(JSOp op, BoolStub stub, jsbytecode *target, JSOp fused);
     void jsop_pos();
 
--- a/js/src/methodjit/FastOps.cpp
+++ b/js/src/methodjit/FastOps.cpp
@@ -1845,19 +1845,44 @@ mjit::Compiler::jsop_getelem_args()
     BarrierState barrier = testBarrier(typeReg, dataReg, false);
 
     stubcc.rejoin(Changes(2));
 
     finishBarrier(barrier, REJOIN_FALLTHROUGH, 0);
 }
 
 #ifdef JS_METHODJIT_TYPED_ARRAY
-void
+bool
 mjit::Compiler::jsop_getelem_typed(int atype)
 {
+    // Unlike dense arrays, the types of elements in typed arrays are not
+    // guaranteed to be present in the object's type, and we need to use
+    // knowledge about the possible contents of the array vs. the types
+    // that have been read out of it to figure out how to do the load.
+
+    //                          Array contents
+    //                   Float     Uint32         Int32
+    // Observed types
+    //
+    // {int}             XXX       reg pair+test  reg
+    // {int,float}       FP reg    FP reg         reg pair
+    // {X,int}           XXX       reg pair+test  reg pair
+    // {X,int,float}     reg pair  reg pair       reg pair
+    // {X}               XXX       XXX            XXX
+
+    // Reject entries marked 'XXX' above, and compile a normal GETELEM.
+    types::TypeSet *pushedTypes = pushedTypeSet(0);
+    if (atype == TypedArray::TYPE_FLOAT32 || atype == TypedArray::TYPE_FLOAT64) {
+        if (!pushedTypes->hasType(types::Type::DoubleType()))
+            return false;
+    } else {
+        if (!pushedTypes->hasType(types::Type::Int32Type()))
+            return false;
+    }
+
     FrameEntry *obj = frame.peek(-2);
     FrameEntry *id = frame.peek(-1);
 
     // We might not know whether this is an object, but if it's an object we
     // know it is a typed array.
     if (!obj->isTypeKnown()) {
         Jump guard = frame.testObject(Assembler::NotEqual, obj);
         stubcc.linkExit(guard, Uses(2));
@@ -1932,35 +1957,41 @@ mjit::Compiler::jsop_getelem_typed(int a
         frame.unpinReg(objReg);
     else
         frame.freeReg(objReg);
     if (!key.isConstant())
         frame.unpinReg(key.reg());
     if (tempReg.isSet())
         frame.freeReg(tempReg.reg());
 
+    if (atype == TypedArray::TYPE_UINT32 &&
+        !pushedTypes->hasType(types::Type::DoubleType())) {
+        Jump isDouble = masm.testDouble(Assembler::Equal, typeReg.reg());
+        stubcc.linkExit(isDouble, Uses(2));
+    }
+
     stubcc.leave();
     OOL_STUBCALL(stubs::GetElem, REJOIN_FALLTHROUGH);
 
     frame.popn(2);
 
     BarrierState barrier;
     if (dataReg.isFPReg()) {
         frame.pushDouble(dataReg.fpreg());
     } else if (typeReg.isSet()) {
         frame.pushRegs(typeReg.reg(), dataReg.reg(), knownPushedType(0));
-        if (hasTypeBarriers(PC))
-            barrier = testBarrier(typeReg.reg(), dataReg.reg(), false);
     } else {
         JS_ASSERT(type == JSVAL_TYPE_INT32);
         frame.pushTypedPayload(JSVAL_TYPE_INT32, dataReg.reg());
     }
     stubcc.rejoin(Changes(2));
 
     finishBarrier(barrier, REJOIN_FALLTHROUGH, 0);
+
+    return true;
 }
 #endif /* JS_METHODJIT_TYPED_ARRAY */
 
 bool
 mjit::Compiler::jsop_getelem(bool isCall)
 {
     FrameEntry *obj = frame.peek(-2);
     FrameEntry *id = frame.peek(-1);
@@ -1989,23 +2020,23 @@ mjit::Compiler::jsop_getelem(bool isCall
             // Inline dense array path.
             bool packed = !types->hasObjectFlags(cx, types::OBJECT_FLAG_NON_PACKED_ARRAY);
             jsop_getelem_dense(packed);
             return true;
         }
 
 #ifdef JS_METHODJIT_TYPED_ARRAY
         if (obj->mightBeType(JSVAL_TYPE_OBJECT) &&
-            !types->hasObjectFlags(cx, types::OBJECT_FLAG_NON_TYPED_ARRAY) &&
-            pushedTypeSet(0)->baseFlags() != 0) {
+            !types->hasObjectFlags(cx, types::OBJECT_FLAG_NON_TYPED_ARRAY)) {
             // Inline typed array path.
             int atype = types->getTypedArrayType(cx);
             if (atype != TypedArray::TYPE_MAX) {
-                jsop_getelem_typed(atype);
-                return true;
+                if (jsop_getelem_typed(atype))
+                    return true;
+                // Fallthrough to the normal GETELEM path.
             }
         }
 #endif
     }
 
     frame.forgetMismatchedObject(obj);
 
     GetElementICInfo ic = GetElementICInfo(JSOp(*PC));