diff --git a/src/maglev/arm64/maglev-assembler-arm64-inl.h b/src/maglev/arm64/maglev-assembler-arm64-inl.h index 2995ca68b6..672ac1e19c 100644 --- a/src/maglev/arm64/maglev-assembler-arm64-inl.h +++ b/src/maglev/arm64/maglev-assembler-arm64-inl.h @@ -6,6 +6,7 @@ #define V8_MAGLEV_ARM64_MAGLEV_ASSEMBLER_ARM64_INL_H_ #include "src/codegen/macro-assembler-inl.h" +#include "src/compiler/compilation-dependencies.h" #include "src/maglev/maglev-assembler.h" #include "src/maglev/maglev-basic-block.h" #include "src/maglev/maglev-code-gen-state.h" @@ -335,7 +336,7 @@ inline void MaglevAssembler::LoadBoundedSizeFromObject(Register result, int offset) { Move(result, FieldMemOperand(object, offset)); #ifdef V8_ENABLE_SANDBOX - Lsl(result, result, kBoundedSizeShift); + Lsr(result, result, kBoundedSizeShift); #endif // V8_ENABLE_SANDBOX } @@ -451,6 +452,25 @@ inline void MaglevAssembler::SignExtend32To64Bits(Register dst, Register src) { Mov(dst, Operand(src.W(), SXTW)); } +template +inline void MaglevAssembler::DeoptIfBufferDetached(Register array, + Register scratch, + NodeT* node) { + if (!code_gen_state() + ->broker() + ->dependencies() + ->DependOnArrayBufferDetachingProtector()) { + // A detached buffer leads to megamorphic feedback, so we won't have a deopt + // loop if we deopt here. + LoadTaggedPointerField( + scratch, FieldMemOperand(array, JSArrayBufferView::kBufferOffset)); + LoadTaggedPointerField( + scratch, FieldMemOperand(scratch, JSArrayBuffer::kBitFieldOffset)); + Tst(scratch.W(), Immediate(JSArrayBuffer::WasDetachedBit::kMask)); + EmitEagerDeoptIf(ne, DeoptimizeReason::kArrayBufferWasDetached, node); + } +} + inline void MaglevAssembler::CompareInt32(Register src1, Register src2) { Cmp(src1.W(), src2.W()); } diff --git a/src/maglev/arm64/maglev-ir-arm64.cc b/src/maglev/arm64/maglev-ir-arm64.cc index 9e80891d7e..b7dcebfb40 100644 --- a/src/maglev/arm64/maglev-ir-arm64.cc +++ b/src/maglev/arm64/maglev-ir-arm64.cc @@ -1793,7 +1793,10 @@ void LoadSignedIntTypedArrayElement::GenerateCode( } UseScratchRegisterScope temps(masm); - Register data_pointer = temps.AcquireX(); + Register scratch = temps.AcquireX(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; int element_size = ElementsKindSize(elements_kind_); __ BuildTypedArrayDataPointer(data_pointer, object); __ Add(data_pointer, data_pointer, Operand(index, LSL, element_size / 2)); @@ -1820,7 +1823,10 @@ void LoadUnsignedIntTypedArrayElement::GenerateCode( } UseScratchRegisterScope temps(masm); - Register data_pointer = temps.AcquireX(); + Register scratch = temps.AcquireX(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; int element_size = ElementsKindSize(elements_kind_); __ BuildTypedArrayDataPointer(data_pointer, object); __ Add(data_pointer, data_pointer, Operand(index, LSL, element_size / 2)); @@ -1847,7 +1853,10 @@ void LoadDoubleTypedArrayElement::GenerateCode(MaglevAssembler* masm, } UseScratchRegisterScope temps(masm); - Register data_pointer = temps.AcquireX(); + Register scratch = temps.AcquireX(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; __ BuildTypedArrayDataPointer(data_pointer, object); switch (elements_kind_) { case FLOAT32_ELEMENTS: diff --git a/src/maglev/maglev-assembler.h b/src/maglev/maglev-assembler.h index 84f1ad26e7..61922dae33 100644 --- a/src/maglev/maglev-assembler.h +++ b/src/maglev/maglev-assembler.h @@ -161,6 +161,10 @@ class MaglevAssembler : public MacroAssembler { inline void SignExtend32To64Bits(Register dst, Register src); + template + inline void DeoptIfBufferDetached(Register array, Register scratch, + NodeT* node); + inline void CompareInt32(Register src1, Register src2); inline void Jump(Label* target); diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h index 68063379a1..bbd1a8ef87 100644 --- a/src/maglev/maglev-ir.h +++ b/src/maglev/maglev-ir.h @@ -4182,8 +4182,12 @@ class LoadSignedIntTypedArrayElement elements_kind == INT32_ELEMENTS); } - static constexpr OpProperties kProperties = - OpProperties::Reading() | OpProperties::Int32(); + // TODO(dmercadier): We should have 2 version of this Node: one that never + // deopts because it's emitted behind a DependOnArrayBufferDetachingProtector + // check, and one that can deopt. + static constexpr OpProperties kProperties = OpProperties::EagerDeopt() | + OpProperties::Reading() | + OpProperties::Int32(); static constexpr typename Base::InputTypes kInputTypes{ ValueRepresentation::kTagged, ValueRepresentation::kUint32}; @@ -4215,8 +4219,12 @@ class LoadUnsignedIntTypedArrayElement elements_kind == UINT32_ELEMENTS); } - static constexpr OpProperties kProperties = - OpProperties::Reading() | OpProperties::Uint32(); + // TODO(dmercadier): We should have 2 version of this Node: one that never + // deopts because it's emitted behind a DependOnArrayBufferDetachingProtector + // check, and one that can deopt. + static constexpr OpProperties kProperties = OpProperties::EagerDeopt() | + OpProperties::Reading() | + OpProperties::Uint32(); static constexpr typename Base::InputTypes kInputTypes{ ValueRepresentation::kTagged, ValueRepresentation::kUint32}; @@ -4245,8 +4253,12 @@ class LoadDoubleTypedArrayElement elements_kind == FLOAT64_ELEMENTS); } - static constexpr OpProperties kProperties = - OpProperties::Reading() | OpProperties::Float64(); + // TODO(dmercadier): We should have 2 version of this Node: one that never + // deopts because it's emitted behind a DependOnArrayBufferDetachingProtector + // check, and one that can deopt. + static constexpr OpProperties kProperties = OpProperties::EagerDeopt() | + OpProperties::Reading() | + OpProperties::Float64(); static constexpr typename Base::InputTypes kInputTypes{ ValueRepresentation::kTagged, ValueRepresentation::kUint32}; diff --git a/src/maglev/x64/maglev-assembler-x64-inl.h b/src/maglev/x64/maglev-assembler-x64-inl.h index 49d1295d5a..25e8a58a4d 100644 --- a/src/maglev/x64/maglev-assembler-x64-inl.h +++ b/src/maglev/x64/maglev-assembler-x64-inl.h @@ -11,6 +11,7 @@ #include "src/codegen/interface-descriptors-inl.h" #include "src/codegen/macro-assembler-inl.h" +#include "src/compiler/compilation-dependencies.h" #include "src/maglev/maglev-assembler.h" #include "src/maglev/maglev-basic-block.h" #include "src/maglev/maglev-code-gen-state.h" @@ -339,6 +340,25 @@ inline void MaglevAssembler::SignExtend32To64Bits(Register dst, Register src) { movsxlq(dst, src); } +template +inline void MaglevAssembler::DeoptIfBufferDetached(Register array, + Register scratch, + NodeT* node) { + if (!code_gen_state() + ->broker() + ->dependencies() + ->DependOnArrayBufferDetachingProtector()) { + // A detached buffer leads to megamorphic feedback, so we won't have a deopt + // loop if we deopt here. + LoadTaggedPointerField( + scratch, FieldOperand(array, JSArrayBufferView::kBufferOffset)); + LoadTaggedPointerField( + scratch, FieldOperand(scratch, JSArrayBuffer::kBitFieldOffset)); + testl(scratch, Immediate(JSArrayBuffer::WasDetachedBit::kMask)); + EmitEagerDeoptIf(not_zero, DeoptimizeReason::kArrayBufferWasDetached, node); + } +} + inline void MaglevAssembler::CompareInt32(Register src1, Register src2) { cmpl(src1, src2); } diff --git a/src/maglev/x64/maglev-ir-x64.cc b/src/maglev/x64/maglev-ir-x64.cc index d8a23362c6..1b21c869d5 100644 --- a/src/maglev/x64/maglev-ir-x64.cc +++ b/src/maglev/x64/maglev-ir-x64.cc @@ -1078,6 +1078,7 @@ void LoadSignedIntTypedArrayElement::GenerateCode( Register object = ToRegister(object_input()); Register index = ToRegister(index_input()); Register result_reg = ToRegister(result()); + Register scratch = general_temporaries().PopFirst(); __ AssertNotSmi(object); if (v8_flags.debug_code) { @@ -1085,7 +1086,9 @@ void LoadSignedIntTypedArrayElement::GenerateCode( __ Assert(equal, AbortReason::kUnexpectedValue); } - Register data_pointer = general_temporaries().PopFirst(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; __ BuildTypedArrayDataPointer(data_pointer, object); int element_size = ElementsKindSize(elements_kind_); __ LoadSignedField( @@ -1105,6 +1108,7 @@ void LoadUnsignedIntTypedArrayElement::GenerateCode( Register object = ToRegister(object_input()); Register index = ToRegister(index_input()); Register result_reg = ToRegister(result()); + Register scratch = general_temporaries().PopFirst(); __ AssertNotSmi(object); if (v8_flags.debug_code) { @@ -1112,7 +1116,9 @@ void LoadUnsignedIntTypedArrayElement::GenerateCode( __ Assert(equal, AbortReason::kUnexpectedValue); } - Register data_pointer = general_temporaries().PopFirst(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; int element_size = ElementsKindSize(elements_kind_); __ BuildTypedArrayDataPointer(data_pointer, object); __ LoadUnsignedField( @@ -1132,13 +1138,17 @@ void LoadDoubleTypedArrayElement::GenerateCode(MaglevAssembler* masm, Register object = ToRegister(object_input()); Register index = ToRegister(index_input()); DoubleRegister result_reg = ToDoubleRegister(result()); + Register scratch = general_temporaries().PopFirst(); + __ AssertNotSmi(object); if (v8_flags.debug_code) { __ CmpObjectType(object, JS_TYPED_ARRAY_TYPE, kScratchRegister); __ Assert(equal, AbortReason::kUnexpectedValue); } - Register data_pointer = general_temporaries().PopFirst(); + __ DeoptIfBufferDetached(object, scratch, this); + + Register data_pointer = scratch; __ BuildTypedArrayDataPointer(data_pointer, object); switch (elements_kind_) { case FLOAT32_ELEMENTS: diff --git a/test/mjsunit/maglev/regress/regress-1405651.js b/test/mjsunit/maglev/regress/regress-1405651.js new file mode 100644 index 0000000000..82113868d7 --- /dev/null +++ b/test/mjsunit/maglev/regress/regress-1405651.js @@ -0,0 +1,55 @@ +// Copyright 2023 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. +// +// Flags: --allow-natives-syntax --harmony --no-always-turbofan --maglev + +// TODO(dmercadier): re-enable this test once on-heap Typed Arrays are +// re-supported by Maglev. +/* +function assertMaglevved(f) { + assertTrue(isMaglevved(f)); +} + +function f(x) { + return x[2]; +} + +let buff = new ArrayBuffer(1024); +let arr = new Int32Array(buff); +arr[2] = 42; + +%PrepareFunctionForOptimization(f); +assertEquals(42, f(arr)); + +%OptimizeMaglevOnNextCall(f); +assertEquals(42, f(arr)); + +assertMaglevved(f); +// Detaching {buff} will cause {f} to deoptimize thanks to the protector. +buff.transfer(); +assertUnoptimized(f); + +assertEquals(undefined, f(arr)); + +let buff2 = new ArrayBuffer(1024); +let arr2 = new Int32Array(buff2); +arr2[2] = 42; + +// Re-optimizing {f} (which a fresh feedback), now that the protector for +// detached array buffer doesn't hold anymore. +%ClearFunctionFeedback(f); +%PrepareFunctionForOptimization(f); +assertEquals(42, f(arr2)); +%OptimizeMaglevOnNextCall(f); +assertEquals(42, f(arr2)); +assertMaglevved(f); + +// The protector doesn't hold anymore, so detaching buff2 shouldn't deopt {f}. +buff2.transfer(); +assertMaglevved(f); + +// The runtime check will call a deopt since {buff2} has been detached. +assertEquals(undefined, f(arr2)); +assertUnoptimized(f); +*/