Bug 1526315: Non-integer typed array access can lead to repeated bailouts r=iain
authorWill Hawkins <whawkins@mozilla.com>
Mon, 11 Mar 2019 21:16:19 +0000
changeset 524447 d8098dde10d83f0f9b37bda5ffd37a42317d15b3
parent 524446 8620e56a5f3858391b42f51684ff735699895049
child 524448 95274fd66e5bea637cc26d7047aef7f96d5e19a7
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersiain
bugs1526315
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1526315: Non-integer typed array access can lead to repeated bailouts r=iain Differential Revision: https://phabricator.services.mozilla.com/D21790
js/public/TrackedOptimizationInfo.h
js/src/jit/BaselineIC.cpp
js/src/jit/BaselineIC.h
js/src/jit/BaselineInspector.cpp
js/src/jit/BaselineInspector.h
js/src/jit/IonBuilder.cpp
js/src/jit/IonTypes.h
--- a/js/public/TrackedOptimizationInfo.h
+++ b/js/public/TrackedOptimizationInfo.h
@@ -109,16 +109,17 @@ namespace JS {
   _(NoProtoFound)                                 \
   _(MultiProtoPaths)                              \
   _(NonWritableProperty)                          \
   _(ProtoIndexedProps)                            \
   _(ArrayBadFlags)                                \
   _(ArrayDoubleConversion)                        \
   _(ArrayRange)                                   \
   _(ArraySeenNegativeIndex)                       \
+  _(ArraySeenNonIntegerIndex)                     \
   _(TypedObjectHasDetachedBuffer)                 \
   _(TypedObjectArrayRange)                        \
   _(AccessNotDense)                               \
   _(AccessNotSimdObject)                          \
   _(AccessNotTypedObject)                         \
   _(AccessNotTypedArray)                          \
   _(AccessNotString)                              \
   _(OperandNotString)                             \
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -2024,16 +2024,24 @@ static bool DoGetElemFallback(JSContext*
 
   // GetElem operations which could access negative indexes generally can't
   // be optimized without the potential for bailouts, as we can't statically
   // determine that an object has no properties on such indexes.
   if (rhs.isNumber() && rhs.toNumber() < 0) {
     stub->noteNegativeIndex();
   }
 
+  // GetElem operations which could access non-integer indexes generally can't
+  // be optimized without the potential for bailouts.
+  int32_t representable;
+  if (rhs.isNumber() && rhs.isDouble() &&
+      !mozilla::NumberEqualsInt32(rhs.toDouble(), &representable)) {
+    stub->setSawNonIntegerIndex();
+  }
+
   return true;
 }
 
 static bool DoGetElemSuperFallback(JSContext* cx, BaselineFrame* frame,
                                    ICGetElem_Fallback* stub, HandleValue lhs,
                                    HandleValue rhs, HandleValue receiver,
                                    MutableHandleValue res) {
   stub->incrementEnteredCount();
@@ -2094,16 +2102,24 @@ static bool DoGetElemSuperFallback(JSCon
 
   // GetElem operations which could access negative indexes generally can't
   // be optimized without the potential for bailouts, as we can't statically
   // determine that an object has no properties on such indexes.
   if (rhs.isNumber() && rhs.toNumber() < 0) {
     stub->noteNegativeIndex();
   }
 
+  // GetElem operations which could access non-integer indexes generally can't
+  // be optimized without the potential for bailouts.
+  int32_t representable;
+  if (rhs.isNumber() && rhs.isDouble() &&
+      !mozilla::NumberEqualsInt32(rhs.toDouble(), &representable)) {
+    stub->setSawNonIntegerIndex();
+  }
+
   return true;
 }
 
 typedef bool (*DoGetElemFallbackFn)(JSContext*, BaselineFrame*,
                                     ICGetElem_Fallback*, HandleValue,
                                     HandleValue, MutableHandleValue);
 static const VMFunction DoGetElemFallbackInfo =
     FunctionInfo<DoGetElemFallbackFn>(DoGetElemFallback, "DoGetElemFallback",
--- a/js/src/jit/BaselineIC.h
+++ b/js/src/jit/BaselineIC.h
@@ -1656,21 +1656,25 @@ class ICToBool_Fallback : public ICFallb
 
 class ICGetElem_Fallback : public ICMonitoredFallbackStub {
   friend class ICStubSpace;
 
   explicit ICGetElem_Fallback(JitCode* stubCode)
       : ICMonitoredFallbackStub(ICStub::GetElem_Fallback, stubCode) {}
 
   static const uint16_t EXTRA_NEGATIVE_INDEX = 0x1;
+  static const uint16_t SAW_NON_INTEGER_INDEX_BIT = 0x2;
 
  public:
   void noteNegativeIndex() { extra_ |= EXTRA_NEGATIVE_INDEX; }
   bool hasNegativeIndex() const { return extra_ & EXTRA_NEGATIVE_INDEX; }
 
+  void setSawNonIntegerIndex() { extra_ |= SAW_NON_INTEGER_INDEX_BIT; }
+  bool sawNonIntegerIndex() const { return extra_ & SAW_NON_INTEGER_INDEX_BIT; }
+
   // Compiler for this stub kind.
   class Compiler : public ICStubCompiler {
    protected:
     CodeOffset bailoutReturnOffset_;
     bool hasReceiver_;
     MOZ_MUST_USE bool generateStubCode(MacroAssembler& masm) override;
     void postGenerateStubCode(MacroAssembler& masm,
                               Handle<JitCode*> code) override;
--- a/js/src/jit/BaselineInspector.cpp
+++ b/js/src/jit/BaselineInspector.cpp
@@ -716,16 +716,29 @@ MIRType BaselineInspector::expectedBinar
     if (TryToSpecializeBinaryArithOp(stubs, 2, &result)) {
       return result;
     }
   }
 
   return MIRType::None;
 }
 
+bool BaselineInspector::hasSeenNonIntegerIndex(jsbytecode* pc) {
+  if (!hasICScript()) {
+    return false;
+  }
+
+  const ICEntry& entry = icEntryFromPC(pc);
+  ICStub* stub = entry.fallbackStub();
+
+  MOZ_ASSERT(stub->isGetElem_Fallback());
+
+  return stub->toGetElem_Fallback()->sawNonIntegerIndex();
+}
+
 bool BaselineInspector::hasSeenNegativeIndexGetElement(jsbytecode* pc) {
   if (!hasICScript()) {
     return false;
   }
 
   const ICEntry& entry = icEntryFromPC(pc);
   ICStub* stub = entry.fallbackStub();
 
@@ -757,17 +770,16 @@ bool BaselineInspector::hasSeenDoubleRes
   const ICEntry& entry = icEntryFromPC(pc);
   ICStub* stub = entry.fallbackStub();
 
   MOZ_ASSERT(stub->isUnaryArith_Fallback() || stub->isBinaryArith_Fallback());
 
   if (stub->isUnaryArith_Fallback()) {
     return stub->toUnaryArith_Fallback()->sawDoubleResult();
   }
-
   return stub->toBinaryArith_Fallback()->sawDoubleResult();
 }
 
 JSObject* BaselineInspector::getTemplateObject(jsbytecode* pc) {
   if (!hasICScript()) {
     return nullptr;
   }
 
--- a/js/src/jit/BaselineInspector.h
+++ b/js/src/jit/BaselineInspector.h
@@ -90,16 +90,17 @@ class BaselineInspector {
   }
 
   MIRType expectedResultType(jsbytecode* pc);
   MCompare::CompareType expectedCompareType(jsbytecode* pc);
   MIRType expectedBinaryArithSpecialization(jsbytecode* pc);
   MIRType expectedPropertyAccessInputType(jsbytecode* pc);
 
   bool hasSeenNegativeIndexGetElement(jsbytecode* pc);
+  bool hasSeenNonIntegerIndex(jsbytecode* pc);
   bool hasSeenAccessedGetter(jsbytecode* pc);
   bool hasSeenDoubleResult(jsbytecode* pc);
 
   MOZ_MUST_USE bool isOptimizableConstStringSplit(jsbytecode* pc,
                                                   JSString** strOut,
                                                   JSString** sepOut,
                                                   ArrayObject** objOut);
   JSObject* getTemplateObject(jsbytecode* pc);
--- a/js/src/jit/IonBuilder.cpp
+++ b/js/src/jit/IonBuilder.cpp
@@ -8884,18 +8884,23 @@ AbortReasonOr<Ok> IonBuilder::getElemTry
   bool hasExtraIndexedProperty;
   MOZ_TRY_VAR(hasExtraIndexedProperty,
               ElementAccessHasExtraIndexedProperty(this, obj));
   if (hasExtraIndexedProperty && failedBoundsCheck_) {
     trackOptimizationOutcome(TrackedOutcome::ProtoIndexedProps);
     return Ok();
   }
 
-  // Don't generate a fast path if this pc has seen negative indexes accessed,
-  // which will not appear to be extra indexed properties.
+  // Don't generate a fast path if this pc has seen negative
+  // or floating-point indexes accessed which will not appear
+  // to be extra indexed properties.
+  if (inspector->hasSeenNonIntegerIndex(pc)) {
+    trackOptimizationOutcome(TrackedOutcome::ArraySeenNonIntegerIndex);
+    return Ok();
+  }
   if (inspector->hasSeenNegativeIndexGetElement(pc)) {
     trackOptimizationOutcome(TrackedOutcome::ArraySeenNegativeIndex);
     return Ok();
   }
 
   MOZ_TRY(jsop_getelem_dense(obj, index));
 
   trackOptimizationSuccess();
@@ -8909,16 +8914,28 @@ AbortReasonOr<Ok> IonBuilder::getElemTry
   MOZ_ASSERT(*emitted == false);
 
   Scalar::Type arrayType;
   if (!ElementAccessIsTypedArray(constraints(), obj, index, &arrayType)) {
     trackOptimizationOutcome(TrackedOutcome::AccessNotTypedArray);
     return Ok();
   }
 
+  // Don't generate a fast path if this pc has seen negative
+  // or floating-point indexes accessed which will not appear
+  // to be extra indexed properties.
+  if (inspector->hasSeenNonIntegerIndex(pc)) {
+    trackOptimizationOutcome(TrackedOutcome::ArraySeenNonIntegerIndex);
+    return Ok();
+  }
+  if (inspector->hasSeenNegativeIndexGetElement(pc)) {
+    trackOptimizationOutcome(TrackedOutcome::ArraySeenNegativeIndex);
+    return Ok();
+  }
+
   // Emit typed getelem variant.
   MOZ_TRY(jsop_getelem_typed(obj, index, arrayType));
 
   trackOptimizationSuccess();
   *emitted = true;
   return Ok();
 }
 
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -104,16 +104,19 @@ enum BailoutKind {
   Bailout_MonitorTypes,
 
   // We hit a hole in an array.
   Bailout_Hole,
 
   // Array access with negative index
   Bailout_NegativeIndex,
 
+  // Array access with non integer index
+  Bailout_NonIntegerIndex,
+
   // Pretty specific case:
   //  - need a type barrier on a property write
   //  - all but one of the observed types have property types that reflect
   //    the value
   //  - we need to guard that we're not given an object of that one other type
   //    also used for the unused GuardClass instruction
   Bailout_ObjectIdentityOrTypeGuard,
 
@@ -205,16 +208,18 @@ inline const char* BailoutKindString(Bai
     case Bailout_TypeBarrierV:
       return "Bailout_TypeBarrierV";
     case Bailout_MonitorTypes:
       return "Bailout_MonitorTypes";
     case Bailout_Hole:
       return "Bailout_Hole";
     case Bailout_NegativeIndex:
       return "Bailout_NegativeIndex";
+    case Bailout_NonIntegerIndex:
+      return "Bailout_NonIntegerIndex";
     case Bailout_ObjectIdentityOrTypeGuard:
       return "Bailout_ObjectIdentityOrTypeGuard";
     case Bailout_NonInt32Input:
       return "Bailout_NonInt32Input";
     case Bailout_NonNumericInput:
       return "Bailout_NonNumericInput";
     case Bailout_NonBooleanInput:
       return "Bailout_NonBooleanInput";