Bug 998059 - Correctly optimize accesses to typed array lengths, and allow length and other properties on typed array prototypes to be redefined, r=jandem,waldo.
authorBrian Hackett <bhackett1024@gmail.com>
Thu, 05 Jun 2014 11:02:33 -0600
changeset 186862 b1d854aec0e3f4c9968130280f3973e3eb5eb70e
parent 186861 d0737ba3f4c9c612f67b9e26ce95e53818d029ed
child 186863 b0dbdb35eacef702d9ee3759ee9b724f6cea62fb
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersjandem, waldo
bugs998059
milestone32.0a1
Bug 998059 - Correctly optimize accesses to typed array lengths, and allow length and other properties on typed array prototypes to be redefined, r=jandem,waldo.
js/src/jit-test/tests/ion/bug998059.js
js/src/jit/BaselineIC.cpp
js/src/jit/BaselineIC.h
js/src/jit/CodeGenerator.cpp
js/src/jit/IonBuilder.cpp
js/src/jit/IonBuilder.h
js/src/jit/MCallOptimize.cpp
js/src/vm/TypedArrayObject.cpp
js/src/vm/TypedArrayObject.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug998059.js
@@ -0,0 +1,25 @@
+// Test various ways of changing the behavior of |typedArray.length|.
+
+function addLengthProperty() {
+  var x = new Uint16Array();
+  Object.defineProperty(x, "length", {value:1});
+  for (var i = 0; i < 5; i++)
+    assertEq(x.length, 1);
+}
+addLengthProperty();
+
+function changePrototype() {
+  var x = new Uint16Array();
+  x.__proto__ = [0];
+  for (var i = 0; i < 5; i++)
+    assertEq(x.length, 1);
+}
+changePrototype();
+
+function redefineLengthProperty() {
+  var x = new Uint16Array();
+  Object.defineProperty(Uint16Array.prototype, "length", {value:1});
+  for (var i = 0; i < 5; i++)
+    assertEq(x.length, 1);
+}
+redefineLengthProperty();
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -6060,28 +6060,16 @@ TryAttachLengthStub(JSContext *cx, JSScr
         if (!newStub)
             return false;
 
         *attached = true;
         stub->addNewStub(newStub);
         return true;
     }
 
-    if (obj->is<TypedArrayObject>() && res.isInt32()) {
-        IonSpew(IonSpew_BaselineIC, "  Generating GetProp(TypedArray.length) stub");
-        ICGetProp_TypedArrayLength::Compiler compiler(cx);
-        ICStub *newStub = compiler.getStub(compiler.getStubSpace(script));
-        if (!newStub)
-            return false;
-
-        *attached = true;
-        stub->addNewStub(newStub);
-        return true;
-    }
-
     if (obj->is<ArgumentsObject>() && res.isInt32()) {
         IonSpew(IonSpew_BaselineIC, "  Generating GetProp(ArgsObj.length %s) stub",
                 obj->is<StrictArgumentsObject>() ? "Strict" : "Normal");
         ICGetProp_ArgumentsLength::Which which = ICGetProp_ArgumentsLength::Normal;
         if (obj->is<StrictArgumentsObject>())
             which = ICGetProp_ArgumentsLength::Strict;
         ICGetProp_ArgumentsLength::Compiler compiler(cx, which);
         ICStub *newStub = compiler.getStub(compiler.getStubSpace(script));
@@ -6510,45 +6498,16 @@ ICGetProp_ArrayLength::Compiler::generat
 
     // Failure case - jump to next stub
     masm.bind(&failure);
     EmitStubGuardFailure(masm);
     return true;
 }
 
 bool
-ICGetProp_TypedArrayLength::Compiler::generateStubCode(MacroAssembler &masm)
-{
-    Label failure;
-    masm.branchTestObject(Assembler::NotEqual, R0, &failure);
-
-    Register scratch = R1.scratchReg();
-
-    // Unbox R0.
-    Register obj = masm.extractObject(R0, ExtractTemp0);
-
-    // Implement the negated version of JSObject::isTypedArray predicate.
-    masm.loadObjClass(obj, scratch);
-    masm.branchPtr(Assembler::Below, scratch, ImmPtr(&TypedArrayObject::classes[0]),
-                   &failure);
-    masm.branchPtr(Assembler::AboveOrEqual, scratch,
-                   ImmPtr(&TypedArrayObject::classes[ScalarTypeDescr::TYPE_MAX]),
-                   &failure);
-
-    // Load length from fixed slot.
-    masm.loadValue(Address(obj, TypedArrayObject::lengthOffset()), R0);
-    EmitReturnFromIC(masm);
-
-    // Failure case - jump to next stub
-    masm.bind(&failure);
-    EmitStubGuardFailure(masm);
-    return true;
-}
-
-bool
 ICGetProp_StringLength::Compiler::generateStubCode(MacroAssembler &masm)
 {
     Label failure;
     masm.branchTestString(Assembler::NotEqual, R0, &failure);
 
     // Unbox string and load its length.
     Register string = masm.extractString(R0, ExtractTemp0);
     masm.loadStringLength(string, string);
--- a/js/src/jit/BaselineIC.h
+++ b/js/src/jit/BaselineIC.h
@@ -406,17 +406,16 @@ class ICEntry
                                 \
     _(BindName_Fallback)        \
                                 \
     _(GetIntrinsic_Fallback)    \
     _(GetIntrinsic_Constant)    \
                                 \
     _(GetProp_Fallback)         \
     _(GetProp_ArrayLength)      \
-    _(GetProp_TypedArrayLength) \
     _(GetProp_Primitive)        \
     _(GetProp_StringLength)     \
     _(GetProp_Native)           \
     _(GetProp_NativePrototype)  \
     _(GetProp_CallScripted)     \
     _(GetProp_CallNative)       \
     _(GetProp_CallNativePrototype)\
     _(GetProp_CallDOMProxyNative)\
@@ -4155,46 +4154,16 @@ class ICGetProp_ArrayLength : public ICS
         {}
 
         ICStub *getStub(ICStubSpace *space) {
             return ICGetProp_ArrayLength::New(space, getStubCode());
         }
     };
 };
 
-// Stub for accessing a typed array's length.
-class ICGetProp_TypedArrayLength : public ICStub
-{
-    friend class ICStubSpace;
-
-    explicit ICGetProp_TypedArrayLength(JitCode *stubCode)
-      : ICStub(GetProp_TypedArrayLength, stubCode)
-    {}
-
-  public:
-    static inline ICGetProp_TypedArrayLength *New(ICStubSpace *space, JitCode *code) {
-        if (!code)
-            return nullptr;
-        return space->allocate<ICGetProp_TypedArrayLength>(code);
-    }
-
-    class Compiler : public ICStubCompiler {
-        bool generateStubCode(MacroAssembler &masm);
-
-      public:
-        explicit Compiler(JSContext *cx)
-          : ICStubCompiler(cx, ICStub::GetProp_TypedArrayLength)
-        {}
-
-        ICStub *getStub(ICStubSpace *space) {
-            return ICGetProp_TypedArrayLength::New(space, getStubCode());
-        }
-    };
-};
-
 // Stub for accessing a property on a primitive's prototype.
 class ICGetProp_Primitive : public ICMonitoredStub
 {
     friend class ICStubSpace;
 
   protected: // Protected to silence Clang warning.
     // Shape of String.prototype/Number.prototype to check for.
     HeapPtrShape protoShape_;
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -3480,17 +3480,17 @@ CodeGenerator::generateBody()
 
             if (!callTraceLIR(i, *iter))
                 return false;
 
             if (!iter->accept(this))
                 return false;
 
 #ifdef DEBUG
-            if (!emitDebugResultChecks(*iter))
+            if (!counts && !emitDebugResultChecks(*iter))
                 return false;
 #endif
         }
         if (masm.oom())
             return false;
 
 #if defined(JS_ION_PERF)
         perfSpewer->endBasicBlock(masm);
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -8162,23 +8162,16 @@ IonBuilder::jsop_length_fastPath()
             current->add(elements);
 
             // Read length.
             MArrayLength *length = MArrayLength::New(alloc(), elements);
             current->add(length);
             current->push(length);
             return true;
         }
-
-        if (objTypes && objTypes->getTypedArrayType() != ScalarTypeDescr::TYPE_MAX) {
-            current->pop();
-            MInstruction *length = addTypedArrayLength(obj);
-            current->push(length);
-            return true;
-        }
     }
 
     return false;
 }
 
 bool
 IonBuilder::jsop_arguments()
 {
@@ -8886,16 +8879,29 @@ IonBuilder::getPropTryCommonGetter(bool 
     pushConstant(ObjectValue(*commonGetter));
 
     current->push(obj);
 
     CallInfo callInfo(alloc(), false);
     if (!callInfo.init(current, 0))
         return false;
 
+    if (commonGetter->isNative()) {
+        InliningStatus status = inlineNativeGetter(callInfo, commonGetter);
+        switch (status) {
+          case InliningStatus_Error:
+            return false;
+          case InliningStatus_NotInlined:
+            break;
+          case InliningStatus_Inlined:
+            *emitted = true;
+            return true;
+        }
+    }
+
     // Inline if we can, otherwise, forget it and just generate a call.
     bool inlineable = false;
     if (commonGetter->isInterpreted()) {
         InliningDecision decision = makeInliningDecision(commonGetter, callInfo);
         switch (decision) {
           case InliningDecision_Error:
             return false;
           case InliningDecision_DontInline:
--- a/js/src/jit/IonBuilder.h
+++ b/js/src/jit/IonBuilder.h
@@ -748,16 +748,17 @@ class IonBuilder : public MIRGenerator
     InliningStatus inlineBailout(CallInfo &callInfo);
     InliningStatus inlineAssertFloat32(CallInfo &callInfo);
 
     // Bind function.
     InliningStatus inlineBoundFunction(CallInfo &callInfo, JSFunction *target);
 
     // Main inlining functions
     InliningStatus inlineNativeCall(CallInfo &callInfo, JSFunction *target);
+    InliningStatus inlineNativeGetter(CallInfo &callInfo, JSFunction *target);
     bool inlineScriptedCall(CallInfo &callInfo, JSFunction *target);
     InliningStatus inlineSingleCall(CallInfo &callInfo, JSFunction *target);
 
     // Call functions
     InliningStatus inlineCallsite(ObjectVector &targets, ObjectVector &originals,
                                   bool lambda, CallInfo &callInfo);
     bool inlineCalls(CallInfo &callInfo, ObjectVector &targets, ObjectVector &originals,
                      BoolVector &choiceSet, MGetPropertyCache *maybeCache);
--- a/js/src/jit/MCallOptimize.cpp
+++ b/js/src/jit/MCallOptimize.cpp
@@ -201,16 +201,45 @@ IonBuilder::inlineNativeCall(CallInfo &c
 
     // Bound function
     if (native == js::CallOrConstructBoundFunction)
         return inlineBoundFunction(callInfo, target);
 
     return InliningStatus_NotInlined;
 }
 
+IonBuilder::InliningStatus
+IonBuilder::inlineNativeGetter(CallInfo &callInfo, JSFunction *target)
+{
+    JS_ASSERT(target->isNative());
+    JSNative native = target->native();
+
+    if (!optimizationInfo().inlineNative())
+        return InliningStatus_NotInlined;
+
+    types::TemporaryTypeSet *thisTypes = callInfo.thisArg()->resultTypeSet();
+    JS_ASSERT(callInfo.argc() == 0);
+
+    // Try to optimize typed array lengths. There is one getter for each
+    // typed array prototype, and make sure we are accessing the right one
+    // for the type of the instance object.
+    if (thisTypes) {
+        ScalarTypeDescr::Type type = (ScalarTypeDescr::Type) thisTypes->getTypedArrayType();
+        if (type != ScalarTypeDescr::TYPE_MAX &&
+            TypedArrayObject::isOriginalLengthGetter(type, native))
+        {
+            MInstruction *length = addTypedArrayLength(callInfo.thisArg());
+            current->push(length);
+            return InliningStatus_Inlined;
+        }
+    }
+
+    return InliningStatus_NotInlined;
+}
+
 types::TemporaryTypeSet *
 IonBuilder::getInlineReturnTypeSet()
 {
     return bytecodeTypes(pc);
 }
 
 MIRType
 IonBuilder::getInlineReturnType()
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -472,17 +472,17 @@ class TypedArrayObjectTemplate : public 
                                     ThisTypedArrayObject::BufferGetterImpl>(cx, args);
     }
 
     // Define an accessor for a read-only property that invokes a native getter
     static bool
     DefineGetter(JSContext *cx, HandleObject proto, PropertyName *name, Native native)
     {
         RootedId id(cx, NameToId(name));
-        unsigned attrs = JSPROP_SHARED | JSPROP_GETTER | JSPROP_PERMANENT;
+        unsigned attrs = JSPROP_SHARED | JSPROP_GETTER;
 
         Rooted<GlobalObject*> global(cx, cx->compartment()->maybeGlobal());
         JSObject *getter = NewFunction(cx, NullPtr(), native, 0,
                                        JSFunction::NATIVE_FUN, global, NullPtr());
         if (!getter)
             return false;
 
         return DefineNativeProperty(cx, proto, id, UndefinedHandleValue,
@@ -2069,20 +2069,20 @@ bool _typedArray##_lengthGetter(JSContex
 }                                                                                  \
 bool _typedArray##_byteLengthGetter(JSContext *cx, unsigned argc, Value *vp) {     \
     return _typedArray##Object::Getter<_typedArray##Object::byteLengthValue>(cx, argc, vp); \
 }                                                                                  \
 bool _typedArray##_byteOffsetGetter(JSContext *cx, unsigned argc, Value *vp) {     \
     return _typedArray##Object::Getter<_typedArray##Object::byteOffsetValue>(cx, argc, vp); \
 }                                                                                  \
 const JSPropertySpec _typedArray##Object::jsprops[] = {                            \
-    JS_PSG("length", _typedArray##_lengthGetter, JSPROP_PERMANENT),                \
-    JS_PSG("buffer", _typedArray##Object::BufferGetter, JSPROP_PERMANENT),         \
-    JS_PSG("byteLength", _typedArray##_byteLengthGetter, JSPROP_PERMANENT),        \
-    JS_PSG("byteOffset", _typedArray##_byteOffsetGetter, JSPROP_PERMANENT),        \
+    JS_PSG("length", _typedArray##_lengthGetter, 0),                               \
+    JS_PSG("buffer", _typedArray##Object::BufferGetter, 0),                        \
+    JS_PSG("byteLength", _typedArray##_byteLengthGetter, 0),                       \
+    JS_PSG("byteOffset", _typedArray##_byteOffsetGetter, 0),                       \
     JS_PS_END                                                                      \
 };
 
 #define IMPL_TYPED_ARRAY_JSAPI_CONSTRUCTORS(Name,NativeType)                                    \
   JS_FRIEND_API(JSObject *) JS_New ## Name ## Array(JSContext *cx, uint32_t nelements)          \
   {                                                                                             \
       return TypedArrayObjectTemplate<NativeType>::fromLength(cx, nelements);                   \
   }                                                                                             \
@@ -2304,17 +2304,17 @@ js_InitArrayBufferClass(JSContext *cx, H
     {
         return nullptr;
     }
 
     if (!LinkConstructorAndPrototype(cx, ctor, arrayBufferProto))
         return nullptr;
 
     RootedId byteLengthId(cx, NameToId(cx->names().byteLength));
-    unsigned attrs = JSPROP_SHARED | JSPROP_GETTER | JSPROP_PERMANENT;
+    unsigned attrs = JSPROP_SHARED | JSPROP_GETTER;
     JSObject *getter = NewFunction(cx, NullPtr(), ArrayBufferObject::byteLengthGetter, 0,
                                    JSFunction::NATIVE_FUN, global, NullPtr());
     if (!getter)
         return nullptr;
 
     if (!DefineNativeProperty(cx, arrayBufferProto, byteLengthId, UndefinedHandleValue,
                               JS_DATA_TO_FUNC_PTR(PropertyOp, getter), nullptr, attrs))
         return nullptr;
@@ -2323,16 +2323,44 @@ js_InitArrayBufferClass(JSContext *cx, H
         return nullptr;
 
     if (!JS_DefineFunctions(cx, arrayBufferProto, ArrayBufferObject::jsfuncs))
         return nullptr;
 
     return arrayBufferProto;
 }
 
+/* static */ bool
+TypedArrayObject::isOriginalLengthGetter(ScalarTypeDescr::Type type, Native native)
+{
+    switch (type) {
+      case ScalarTypeDescr::TYPE_INT8:
+        return native == Int8Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_UINT8:
+        return native == Uint8Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_UINT8_CLAMPED:
+        return native == Uint8ClampedArray_lengthGetter;
+      case ScalarTypeDescr::TYPE_INT16:
+        return native == Int16Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_UINT16:
+        return native == Uint16Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_INT32:
+        return native == Int32Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_UINT32:
+        return native == Uint32Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_FLOAT32:
+        return native == Float32Array_lengthGetter;
+      case ScalarTypeDescr::TYPE_FLOAT64:
+        return native == Float64Array_lengthGetter;
+      default:
+        MOZ_ASSUME_UNREACHABLE("Unknown TypedArray type");
+        return false;
+    }
+}
+
 const Class DataViewObject::protoClass = {
     "DataViewPrototype",
     JSCLASS_HAS_PRIVATE |
     JSCLASS_HAS_RESERVED_SLOTS(DataViewObject::RESERVED_SLOTS) |
     JSCLASS_HAS_CACHED_PROTO(JSProto_DataView),
     JS_PropertyStub,         /* addProperty */
     JS_DeletePropertyStub,   /* delProperty */
     JS_PropertyStub,         /* getProperty */
@@ -2398,17 +2426,17 @@ DataViewObject::getter(JSContext *cx, un
     return CallNonGenericMethod<is, getterImpl<ValueGetter> >(cx, args);
 }
 
 template<Value ValueGetter(DataViewObject *view)>
 bool
 DataViewObject::defineGetter(JSContext *cx, PropertyName *name, HandleObject proto)
 {
     RootedId id(cx, NameToId(name));
-    unsigned attrs = JSPROP_SHARED | JSPROP_GETTER | JSPROP_PERMANENT;
+    unsigned attrs = JSPROP_SHARED | JSPROP_GETTER;
 
     Rooted<GlobalObject*> global(cx, cx->compartment()->maybeGlobal());
     JSObject *getter = NewFunction(cx, NullPtr(), DataViewObject::getter<ValueGetter>, 0,
                                    JSFunction::NATIVE_FUN, global, NullPtr());
     if (!getter)
         return false;
 
     return DefineNativeProperty(cx, proto, id, UndefinedHandleValue,
--- a/js/src/vm/TypedArrayObject.h
+++ b/js/src/vm/TypedArrayObject.h
@@ -135,16 +135,18 @@ class TypedArrayObject : public ArrayBuf
     /*
      * Byte length above which created typed arrays and data views will have
      * singleton types regardless of the context in which they are created.
      */
     static const uint32_t SINGLETON_TYPE_BYTE_LENGTH = 1024 * 1024 * 10;
 
     static int lengthOffset();
     static int dataOffset();
+
+    static bool isOriginalLengthGetter(ScalarTypeDescr::Type type, Native native);
 };
 
 inline bool
 IsTypedArrayClass(const Class *clasp)
 {
     return &TypedArrayObject::classes[0] <= clasp &&
            clasp < &TypedArrayObject::classes[ScalarTypeDescr::TYPE_MAX];
 }