Bug 1647250 - Fix MArrayLength to check for non-int32 array lengths in Warp. r=evilpie
authorJan de Mooij <jdemooij@mozilla.com>
Tue, 30 Jun 2020 10:45:51 +0000
changeset 538173 d3ba9f8006f6c3b0eba291bf24c2956eac05143e
parent 538172 b0cc716aa5c034ee11a92c5bcd93655b11efd407
child 538174 0023408ef9059cb2bcf039014f2dce798c639185
push id120452
push userjdemooij@mozilla.com
push dateWed, 01 Jul 2020 08:47:53 +0000
treeherderautoland@d3ba9f8006f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1647250
milestone80.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 1647250 - Fix MArrayLength to check for non-int32 array lengths in Warp. r=evilpie Differential Revision: https://phabricator.services.mozilla.com/D81670
js/src/jit-test/tests/warp/non-int32-array-length.js
js/src/jit/BaselineBailouts.cpp
js/src/jit/CodeGenerator.cpp
js/src/jit/IonTypes.h
js/src/jit/Lowering.cpp
js/src/jit/RangeAnalysis.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/warp/non-int32-array-length.js
@@ -0,0 +1,10 @@
+function f(arr, len) {
+    for (var i = 0; i < 2000; i++) {
+        assertEq(arr.length, len);
+    }
+}
+var arr = [0];
+f(arr, 1);
+
+arr.length = 0xffff_ffff;
+f(arr, 0xffff_ffff);
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -1996,16 +1996,17 @@ bool jit::FinishBailoutToBaseline(Baseli
     case Bailout_NonBooleanInput:
     case Bailout_NonObjectInput:
     case Bailout_NonStringInput:
     case Bailout_NonSymbolInput:
     case Bailout_NonBigIntInput:
     case Bailout_Debugger:
     case Bailout_SpecificAtomGuard:
     case Bailout_SpecificSymbolGuard:
+    case Bailout_NonInt32ArrayLength:
       // Do nothing.
       break;
 
     case Bailout_FirstExecution:
       // Do not return directly, as this was not frequent in the first place,
       // thus rely on the check for frequent bailouts to recompile the current
       // script.
       break;
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -7635,18 +7635,29 @@ void CodeGenerator::visitImplicitThis(LI
 void CodeGenerator::visitArrowNewTarget(LArrowNewTarget* lir) {
   Register callee = ToRegister(lir->callee());
   ValueOperand output = ToOutValue(lir);
   masm.loadValue(
       Address(callee, FunctionExtended::offsetOfArrowNewTargetSlot()), output);
 }
 
 void CodeGenerator::visitArrayLength(LArrayLength* lir) {
-  Address length(ToRegister(lir->elements()), ObjectElements::offsetOfLength());
-  masm.load32(length, ToRegister(lir->output()));
+  Register elements = ToRegister(lir->elements());
+  Register output = ToRegister(lir->output());
+
+  Address length(elements, ObjectElements::offsetOfLength());
+  masm.load32(length, output);
+
+  // IonBuilder relies on TI knowing the length fits in int32, but Warp needs to
+  // check this dynamically.
+  if (JitOptions.warpBuilder) {
+    Label bail;
+    masm.branchTest32(Assembler::Signed, output, output, &bail);
+    bailoutFrom(&bail, lir->snapshot());
+  }
 }
 
 static void SetLengthFromIndex(MacroAssembler& masm, const LAllocation* index,
                                const Address& length) {
   if (index->isConstant()) {
     masm.store32(Imm32(ToInt32(index) + 1), length);
   } else {
     Register newLength = ToRegister(index);
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -139,16 +139,19 @@ enum BailoutKind {
   Bailout_NonBigIntInput,
 
   // We hit a |debugger;| statement.
   Bailout_Debugger,
 
   // We hit this code for the first time.
   Bailout_FirstExecution,
 
+  // Array length did not fit in int32.
+  Bailout_NonInt32ArrayLength,
+
   // END Normal bailouts
 
   // Bailouts caused by invalid assumptions based on Baseline code.
   // Causes immediate invalidation.
 
   // Like Bailout_Overflow, but causes immediate invalidation.
   Bailout_OverflowInvalidate,
 
@@ -241,16 +244,18 @@ inline const char* BailoutKindString(Bai
     case Bailout_NonSymbolInput:
       return "Bailout_NonSymbolInput";
     case Bailout_NonBigIntInput:
       return "Bailout_NonBigIntInput";
     case Bailout_Debugger:
       return "Bailout_Debugger";
     case Bailout_FirstExecution:
       return "Bailout_FirstExecution";
+    case Bailout_NonInt32ArrayLength:
+      return "Bailout_NonInt32ArrayLength";
 
     // Bailouts caused by invalid assumptions.
     case Bailout_OverflowInvalidate:
       return "Bailout_OverflowInvalidate";
     case Bailout_DoubleOutput:
       return "Bailout_DoubleOutput";
 
     // Other bailouts.
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -2991,17 +2991,21 @@ void LIRGenerator::visitPostWriteElement
       // Currently, only objects, strings, and bigints can be in the nursery.
       // Other instruction types cannot hold nursery pointers.
       break;
   }
 }
 
 void LIRGenerator::visitArrayLength(MArrayLength* ins) {
   MOZ_ASSERT(ins->elements()->type() == MIRType::Elements);
-  define(new (alloc()) LArrayLength(useRegisterAtStart(ins->elements())), ins);
+  auto* lir = new (alloc()) LArrayLength(useRegisterAtStart(ins->elements()));
+  if (JitOptions.warpBuilder) {
+    assignSnapshot(lir, Bailout_NonInt32ArrayLength);
+  }
+  define(lir, ins);
 }
 
 void LIRGenerator::visitSetArrayLength(MSetArrayLength* ins) {
   MOZ_ASSERT(ins->elements()->type() == MIRType::Elements);
   MOZ_ASSERT(ins->index()->type() == MIRType::Int32);
 
   MOZ_ASSERT(ins->index()->isConstant());
   add(new (alloc()) LSetArrayLength(useRegister(ins->elements()),
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -1764,20 +1764,22 @@ void MLoadUnboxedScalar::computeRange(Te
 
 void MLoadDataViewElement::computeRange(TempAllocator& alloc) {
   // We have an Int32 type and if this is a UInt32 load it may produce a value
   // outside of our range, but we have a bailout to handle those cases.
   setRange(GetArrayBufferViewRange(alloc, storageType()));
 }
 
 void MArrayLength::computeRange(TempAllocator& alloc) {
-  // Array lengths can go up to UINT32_MAX, but we only create MArrayLength
+  // Array lengths can go up to UINT32_MAX. IonBuilder only creates MArrayLength
   // nodes when the value is known to be int32 (see the
-  // OBJECT_FLAG_LENGTH_OVERFLOW flag).
-  setRange(Range::NewUInt32Range(alloc, 0, INT32_MAX));
+  // OBJECT_FLAG_LENGTH_OVERFLOW flag). WarpBuilder does a dynamic check and we
+  // have to return the range pre-bailouts, so use UINT32_MAX for Warp.
+  uint32_t max = JitOptions.warpBuilder ? UINT32_MAX : INT32_MAX;
+  setRange(Range::NewUInt32Range(alloc, 0, max));
 }
 
 void MInitializedLength::computeRange(TempAllocator& alloc) {
   setRange(
       Range::NewUInt32Range(alloc, 0, NativeObject::MAX_DENSE_ELEMENTS_COUNT));
 }
 
 void MArrayBufferViewLength::computeRange(TempAllocator& alloc) {