Bug 880208 - Add UnsafeGet and UnsafeGetImmutable intrinsics r=djvj

js/src/builtin/ParallelArray.js

@@ -377,7 +377,7 @@ function ParallelArrayMap(func, mode) {
   // FIXME(bug 844887): Check |IsCallable(func)|
 
   var self = this;
-  var length = self.shape[0];
+  var length = UnsafeGetImmutableElement(self.shape, 0);
   var buffer = NewDenseArray(length);
 
   parallel: for (;;) { // see ParallelArrayBuild() to explain why for(;;) etc
@@ -432,7 +432,7 @@ function ParallelArrayReduce(func, mode) {
   // FIXME(bug 844887): Check |IsCallable(func)|
 
   var self = this;
-  var length = self.shape[0];
+  var length = UnsafeGetImmutableElement(self.shape, 0);
 
   if (length === 0)
     ThrowError(JSMSG_PAR_ARRAY_REDUCE_EMPTY);
@@ -519,7 +519,7 @@ function ParallelArrayScan(func, mode) {
   // FIXME(bug 844887): Check |IsCallable(func)|
 
   var self = this;
-  var length = self.shape[0];
+  var length = UnsafeGetImmutableElement(self.shape, 0);
 
   if (length === 0)
     ThrowError(JSMSG_PAR_ARRAY_REDUCE_EMPTY);
@@ -726,7 +726,7 @@ function ParallelArrayScatter(targets, defaultValue, conflictFunc, length, mode)
   var self = this;
 
   if (length === undefined)
-    length = self.shape[0];
+    length = UnsafeGetImmutableElement(self.shape, 0);
 
   // The Divide-Scatter-Vector strategy:
   // 1. Slice |targets| array of indices ("scatter-vector") into N
@@ -977,7 +977,7 @@ function ParallelArrayFilter(func, mode) {
   // FIXME(bug 844887): Check |IsCallable(func)|
 
   var self = this;
-  var length = self.shape[0];
+  var length = UnsafeGetImmutableElement(self.shape, 0);
 
   parallel: for (;;) { // see ParallelArrayBuild() to explain why for(;;) etc
     if (ShouldForceSequential())
@@ -1151,6 +1151,11 @@ function ParallelArrayFlatten() {
 function ParallelArrayGet1(i) {
   if (i === undefined)
     return undefined;
+
+  // We could use UnsafeGetImmutableElement here, but I am not doing
+  // so (for the moment) since using the default path enables
+  // bounds-check hoisting, which is (currently) not possible
+  // otherwise.
   return this.buffer[this.offset + i];
 }
 
@@ -1158,27 +1163,25 @@ function ParallelArrayGet1(i) {
  * Specialized variant of get() for two-dimensional case
  */
 function ParallelArrayGet2(x, y) {
-  var xDimension = this.shape[0];
+  var xDimension = UnsafeGetImmutableElement(this.shape, 0);
-  var yDimension = this.shape[1];
+  var yDimension = UnsafeGetImmutableElement(this.shape, 1);
-  if (x === undefined)
+  if (x === undefined || TO_INT32(x) !== x || x >= xDimension)
-    return undefined;
-  if (x >= xDimension)
     return undefined;
   if (y === undefined)
     return NewParallelArray(ParallelArrayView, [yDimension], this.buffer, this.offset + x * yDimension);
-  if (y >= yDimension)
+  if (TO_INT32(y) !== y || y >= yDimension)
     return undefined;
   var offset = y + x * yDimension;
-  return this.buffer[this.offset + offset];
+  return UnsafeGetImmutableElement(this.buffer, this.offset + offset);
 }
 
 /**
  * Specialized variant of get() for three-dimensional case
  */
 function ParallelArrayGet3(x, y, z) {
-  var xDimension = this.shape[0];
+  var xDimension = UnsafeGetImmutableElement(this.shape, 0);
-  var yDimension = this.shape[1];
+  var yDimension = UnsafeGetImmutableElement(this.shape, 1);
-  var zDimension = this.shape[2];
+  var zDimension = UnsafeGetImmutableElement(this.shape, 2);
   if (x === undefined)
     return undefined;
   if (x >= xDimension)
@@ -1194,7 +1197,7 @@ function ParallelArrayGet3(x, y, z) {
   if (z >= zDimension)
     return undefined;
   var offset = z + y*zDimension + x * yDimension * zDimension;
-  return this.buffer[this.offset + offset];
+  return UnsafeGetImmutableElement(this.buffer, this.offset + offset);
 }
 
 /**
@@ -1228,7 +1231,7 @@ function ParallelArrayGetN(...coords) {
 
 /** The length property yields the outermost dimension */
 function ParallelArrayLength() {
-  return this.shape[0];
+  return UnsafeGetImmutableElement(this.shape, 0);
 }
 
 function ParallelArrayToString() {

js/src/ion/IonBuilder.cpp

@@ -6264,8 +6264,11 @@ IonBuilder::jsop_getelem()
     if (ElementAccessIsDenseNative(obj, index)) {
         // Don't generate a fast path if there have been bounds check failures
         // and this access might be on a sparse property.
-        if (!ElementAccessHasExtraIndexedProperty(cx, obj) || !failedBoundsCheck_)
+        if (!ElementAccessHasExtraIndexedProperty(cx, obj) || !failedBoundsCheck_) {
-            return jsop_getelem_dense();
+            MDefinition *id = current->pop();
+            MDefinition *obj = current->pop();
+            return jsop_getelem_dense(GetElem_Normal, obj, id);
+        }
     }
 
     int arrayType = TypedArray::TYPE_MAX;
@@ -6330,11 +6333,8 @@ IonBuilder::jsop_getelem()
 }
 
 bool
-IonBuilder::jsop_getelem_dense()
+IonBuilder::jsop_getelem_dense(GetElemSafety safety, MDefinition *obj, MDefinition *id)
 {
-    MDefinition *id = current->pop();
-    MDefinition *obj = current->pop();
-
     types::StackTypeSet *types = types::TypeScript::BytecodeTypes(script(), pc);
 
     if (JSOp(*pc) == JSOP_CALLELEM && !id->mightBeType(MIRType_String) && types->noConstraints()) {
@@ -6351,6 +6351,7 @@ IonBuilder::jsop_getelem_dense()
     // undefined values have been observed at this access site and the access
     // cannot hit another indexed property on the object or its prototypes.
     bool readOutOfBounds =
+        safety == GetElem_Normal &&
         types->hasType(types::Type::UndefinedType()) &&
         !ElementAccessHasExtraIndexedProperty(cx, obj);
 
@@ -6387,9 +6388,6 @@ IonBuilder::jsop_getelem_dense()
     if (loadDouble)
         elements = addConvertElementsToDoubles(elements);
 
-    MInitializedLength *initLength = MInitializedLength::New(elements);
-    current->add(initLength);
-
     MInstruction *load;
 
     if (!readOutOfBounds) {
@@ -6397,14 +6395,28 @@ IonBuilder::jsop_getelem_dense()
         // in-bounds elements, and the array is packed or its holes are not
         // read. This is the best case: we can separate the bounds check for
         // hoisting.
-        id = addBoundsCheck(id, initLength);
+        switch (safety) {
+          case GetElem_Normal: {
+              MInitializedLength *initLength = MInitializedLength::New(elements);
+              current->add(initLength);
+              id = addBoundsCheck(id, initLength);
+              break;
+          }
 
-        load = MLoadElement::New(elements, id, needsHoleCheck, loadDouble);
+          case GetElem_Unsafe: break;
+          case GetElem_UnsafeImmutable: break;
+        }
+
+        bool knownImmutable = (safety == GetElem_UnsafeImmutable);
+        load = MLoadElement::New(elements, id, needsHoleCheck, loadDouble,
+                                 knownImmutable);
         current->add(load);
     } else {
         // This load may return undefined, so assume that we *can* read holes,
         // or that we can read out-of-bounds accesses. In this case, the bounds
         // check is part of the opcode.
+        MInitializedLength *initLength = MInitializedLength::New(elements);
+        current->add(initLength);
         load = MLoadElementHole::New(elements, id, initLength, needsHoleCheck);
         current->add(load);
 

js/src/ion/IonBuilder.h

@@ -41,6 +41,20 @@ class IonBuilder : public MIRGenerator
         SetElem_Unsafe,
     };
 
+    enum GetElemSafety {
+        // Normal read like a[b]
+        GetElem_Normal,
+
+        // Read due to UnsafeGetElement:
+        // - assumed to be in bounds,
+        GetElem_Unsafe,
+
+        // Read due to UnsafeGetImmutableElement:
+        // - assumed to be in bounds,
+        // - assumed not to alias any stores
+        GetElem_UnsafeImmutable,
+    };
+
     struct DeferredEdge : public TempObject
     {
         MBasicBlock *block;
@@ -391,7 +405,7 @@ class IonBuilder : public MIRGenerator
     bool jsop_intrinsic(HandlePropertyName name);
     bool jsop_bindname(PropertyName *name);
     bool jsop_getelem();
-    bool jsop_getelem_dense();
+    bool jsop_getelem_dense(GetElemSafety safety, MDefinition *object, MDefinition *index);
     bool jsop_getelem_typed(int arrayType);
     bool jsop_getelem_typed_static(bool *psucceeded);
     bool jsop_getelem_string();
@@ -486,6 +500,8 @@ class IonBuilder : public MIRGenerator
     InliningStatus inlineUnsafeSetElement(CallInfo &callInfo);
     bool inlineUnsafeSetDenseArrayElement(CallInfo &callInfo, uint32_t base);
     bool inlineUnsafeSetTypedArrayElement(CallInfo &callInfo, uint32_t base, int arrayType);
+    InliningStatus inlineUnsafeGetElement(CallInfo &callInfo,
+                                          GetElemSafety safety);
     InliningStatus inlineForceSequentialOrInParallelSection(CallInfo &callInfo);
     InliningStatus inlineNewDenseArray(CallInfo &callInfo);
     InliningStatus inlineNewDenseArrayForSequentialExecution(CallInfo &callInfo);

js/src/ion/MCallOptimize.cpp

@@ -90,6 +90,10 @@ IonBuilder::inlineNativeCall(CallInfo &callInfo, JSNative native)
     // Array intrinsics.
     if (native == intrinsic_UnsafeSetElement)
         return inlineUnsafeSetElement(callInfo);
+    if (native == intrinsic_UnsafeGetElement)
+        return inlineUnsafeGetElement(callInfo, GetElem_Unsafe);
+    if (native == intrinsic_UnsafeGetImmutableElement)
+        return inlineUnsafeGetElement(callInfo, GetElem_UnsafeImmutable);
     if (native == intrinsic_NewDenseArray)
         return inlineNewDenseArray(callInfo);
 
@@ -952,9 +956,10 @@ IonBuilder::inlineUnsafeSetElement(CallInfo &callInfo)
     /* Important:
      *
      * Here we inline each of the stores resulting from a call to
-     * %UnsafeSetElement().  It is essential that these stores occur
+     * UnsafeSetElement().  It is essential that these stores occur
      * atomically and cannot be interrupted by a stack or recursion
      * check.  If this is not true, race conditions can occur.
+     * See definition of UnsafeSetElement() for more details.
      */
 
     for (uint32_t base = 0; base < argc; base += 3) {
@@ -1053,6 +1058,30 @@ IonBuilder::inlineUnsafeSetTypedArrayElement(CallInfo &callInfo,
     return true;
 }
 
+IonBuilder::InliningStatus
+IonBuilder::inlineUnsafeGetElement(CallInfo &callInfo,
+                                   GetElemSafety safety)
+{
+    JS_ASSERT(safety != GetElem_Normal);
+
+    uint32_t argc = callInfo.argc();
+    if (argc < 2 || callInfo.constructing())
+        return InliningStatus_NotInlined;
+    const uint32_t obj = 0;
+    const uint32_t index = 1;
+    if (!ElementAccessIsDenseNative(callInfo.getArg(obj),
+                                    callInfo.getArg(index)))
+        return InliningStatus_NotInlined;
+    if (ElementAccessHasExtraIndexedProperty(cx, callInfo.getArg(obj)))
+        return InliningStatus_NotInlined;
+    callInfo.unwrapArgs();
+    if (!jsop_getelem_dense(safety,
+                            callInfo.getArg(obj),
+                            callInfo.getArg(index)))
+        return InliningStatus_Error;
+    return InliningStatus_Inlined;
+}
+
 IonBuilder::InliningStatus
 IonBuilder::inlineForceSequentialOrInParallelSection(CallInfo &callInfo)
 {

js/src/ion/MIR.h

@@ -4510,11 +4510,13 @@ class MLoadElement
 {
     bool needsHoleCheck_;
     bool loadDoubles_;
+    bool knownImmutable_; // load of data that is known to be immutable
 
-    MLoadElement(MDefinition *elements, MDefinition *index, bool needsHoleCheck, bool loadDoubles)
+    MLoadElement(MDefinition *elements, MDefinition *index, bool needsHoleCheck, bool loadDoubles, bool knownImmutable)
       : MBinaryInstruction(elements, index),
         needsHoleCheck_(needsHoleCheck),
-        loadDoubles_(loadDoubles)
+        loadDoubles_(loadDoubles),
+        knownImmutable_(knownImmutable)
     {
         setResultType(MIRType_Value);
         setMovable();
@@ -4526,8 +4528,10 @@ class MLoadElement
     INSTRUCTION_HEADER(LoadElement)
 
     static MLoadElement *New(MDefinition *elements, MDefinition *index,
-                             bool needsHoleCheck, bool loadDoubles) {
+                             bool needsHoleCheck, bool loadDoubles,
-        return new MLoadElement(elements, index, needsHoleCheck, loadDoubles);
+                             bool knownImmutable) {
+        return new MLoadElement(elements, index, needsHoleCheck, loadDoubles,
+                                knownImmutable);
     }
 
     TypePolicy *typePolicy() {
@@ -4549,6 +4553,9 @@ class MLoadElement
         return needsHoleCheck();
     }
     AliasSet getAliasSet() const {
+        if (knownImmutable_)
+            return AliasSet::None();
+
         return AliasSet::Load(AliasSet::Element);
     }
 };

js/src/jscntxt.h

@@ -2360,6 +2360,9 @@ JSBool intrinsic_IsCallable(JSContext *cx, unsigned argc, Value *vp);
 JSBool intrinsic_ThrowError(JSContext *cx, unsigned argc, Value *vp);
 JSBool intrinsic_NewDenseArray(JSContext *cx, unsigned argc, Value *vp);
 JSBool intrinsic_UnsafeSetElement(JSContext *cx, unsigned argc, Value *vp);
+JSBool intrinsic_UnsafeGetElement(JSContext *cx, unsigned argc, Value *vp);
+JSBool intrinsic_UnsafeGetImmutableElement(JSContext *cx, unsigned argc,
+                                           Value *vp);
 JSBool intrinsic_ShouldForceSequential(JSContext *cx, unsigned argc, Value *vp);
 JSBool intrinsic_NewParallelArray(JSContext *cx, unsigned argc, Value *vp);
 

js/src/vm/SelfHosting.cpp

@@ -341,15 +341,26 @@ js::intrinsic_NewDenseArray(JSContext *cx, unsigned argc, Value *vp)
 /*
  * UnsafeSetElement(arr0, idx0, elem0, ..., arrN, idxN, elemN): For
  * each set of (arr, idx, elem) arguments that are passed, performs
- * the assignment |arr[idx] = elem|. |arr| must be either a dense array
+ * the assignment `arr[idx] = elem`. `arr` must be either a dense array
  * or a typed array.
  *
- * If |arr| is a dense array, the index must be an int32 less than the
+ * If `arr` is a dense array, the index must be an int32 less than the
- * initialized length of |arr|. Use |%EnsureDenseResultArrayElements|
+ * initialized length of `arr`. Use `NewDenseAllocatedArray` to ensure
- * to ensure that the initialized length is long enough.
+ * that the initialized length is long enough.
  *
- * If |arr| is a typed array, the index must be an int32 less than the
+ * If `arr` is a typed array, the index must be an int32 less than the
- * length of |arr|.
+ * length of `arr`.
+ *
+ * The reason that `UnsafeSetElement` takes multiple
+ * array/index/element triples is not for convenience but rather for
+ * semantic reasons: there are a few places in the parallel code where
+ * correctness relies on the fact that *all of the assignments occur
+ * or none of them*. This occurs in operations like reduce or fold
+ * which mutate the same data in place. That is, we do not want to
+ * bail out or interrupt in between the individual assignments. To
+ * convey this notion, we place all the assignments together into one
+ * `UnsafeSetElement` call. It is preferable to use multiple calls if
+ * it is not important that the assignments occur all-or-nothing.
  */
 JSBool
 js::intrinsic_UnsafeSetElement(JSContext *cx, unsigned argc, Value *vp)
@@ -380,7 +391,6 @@ js::intrinsic_UnsafeSetElement(JSContext *cx, unsigned argc, Value *vp)
         } else {
             JS_ASSERT(idx < TypedArray::length(arrobj));
             RootedValue tmp(cx, args[elemi]);
-            // XXX: Always non-strict.
             if (!JSObject::setElement(cx, arrobj, arrobj, idx, &tmp, false))
                 return false;
         }
@@ -390,6 +400,46 @@ js::intrinsic_UnsafeSetElement(JSContext *cx, unsigned argc, Value *vp)
     return true;
 }
 
+/*
+ * UnsafeGetElement(arr, idx)=elem:
+ *
+ * Loads an element from an array.  Requires that `arr` be a dense
+ * array and `idx` be in bounds.  In ion compiled code, no bounds
+ * check will be emitted.
+ */
+JSBool
+js::intrinsic_UnsafeGetElement(JSContext *cx, unsigned argc, Value *vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    const uint32_t arri = 0;
+    const uint32_t idxi = 1;
+
+    JS_ASSERT(args[arri].isObject());
+    JS_ASSERT(args[idxi].isInt32());
+
+    RootedObject arrobj(cx, &args[arri].toObject());
+    uint32_t idx = args[idxi].toInt32();
+
+    JS_ASSERT(args[arri].toObject().isNative());
+    JS_ASSERT(idx < arrobj->getDenseInitializedLength());
+    args.rval().set(arrobj->getDenseElement(idx));
+    return true;
+}
+
+/*
+ * UnsafeGetImmutableElement(arr, idx)=elem:
+ *
+ * Same as `UnsafeGetElement(arr, idx)`, except that the array is
+ * known by the self-hosting code to be immutable. Therefore, ion
+ * compilation can reorder this load freely with respect to stores.
+ */
+JSBool
+js::intrinsic_UnsafeGetImmutableElement(JSContext *cx, unsigned argc, Value *vp)
+{
+    return intrinsic_UnsafeGetElement(cx, argc, vp);
+}
+
 /*
  * ParallelTestsShouldPass(): Returns false if we are running in a
  * mode (such as --ion-eager) that is known to cause additional
@@ -468,6 +518,8 @@ const JSFunctionSpec intrinsic_functions[] = {
     JS_FN("NewParallelArray",     intrinsic_NewParallelArray,     3,0),
     JS_FN("NewDenseArray",        intrinsic_NewDenseArray,        1,0),
     JS_FN("UnsafeSetElement",     intrinsic_UnsafeSetElement,     3,0),
+    JS_FN("UnsafeGetElement",     intrinsic_UnsafeGetElement, 2,0),
+    JS_FN("UnsafeGetImmutableElement", intrinsic_UnsafeGetImmutableElement, 2,0),
     JS_FN("ShouldForceSequential", intrinsic_ShouldForceSequential, 0,0),
     JS_FN("ParallelTestsShouldPass", intrinsic_ParallelTestsShouldPass, 0,0),