From e639eafea3d67434d288da4fc932a9e82384ae2b Mon Sep 17 00:00:00 2001 From: Bill Budge Date: Wed, 3 Mar 2021 15:20:31 -0800 Subject: [PATCH] Reland "Reland "Reland "[compiler][wasm] Align Frame slots to value size""" This is a reland of 352b9ecbdb090cbb22ee3362fadae28f86ba6773 The test/fix CL has been merged in, as the fixes to return slot accounting are needed to fix Arm64 issues turned up by the fuzzers: https://chromium-review.googlesource.com/c/v8/v8/+/2644139 The reverted fix for Wasm return slot allocation is added in patchset #2, to avoid fuzzer issues that it fixed: https://chromium-review.googlesource.com/c/v8/v8/+/2683024 TBR=neis@chromium.org Original change's description: > Reland "Reland "[compiler][wasm] Align Frame slots to value size"" > > This is a reland of 1694925c728a1be1b7084028bd656ddfc75f6471 > > Minor fix to linkage for constexpr. > > TBR=ahaas@chromium.org,neis@chromium.org > > Original change's description: > > Reland "[compiler][wasm] Align Frame slots to value size" > > > > This is a reland of cddaf66c371c2433c391434776f31b8771c5ab45 > > > > Original change's description: > > > [compiler][wasm] Align Frame slots to value size > > > > > > - Adds an AlignedSlotAllocator class and tests, to unify slot > > > allocation. This attempts to use alignment holes for smaller > > > values. > > > - Reworks Frame to use the new allocator for stack slots. > > > - Reworks LinkageAllocator to use the new allocator for stack > > > slots and for ARMv7 FP register aliasing. > > > - Fixes the RegisterAllocator to align spill slots. > > > - Fixes InstructionSelector to align spill slots. > > > > > > Bug: v8:9198 > > > > > > Change-Id: Ida148db428be89ef95de748ec5fc0e7b0358f523 > > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2512840 > > > Commit-Queue: Bill Budge > > > Reviewed-by: Georg Neis > > > Reviewed-by: Andreas Haas > > > Cr-Commit-Position: refs/heads/master@{#71644} > > > > Bug: v8:9198 > > Change-Id: Ib91fa6746370c38496706341e12d05c7bf999389 > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2633390 > > Commit-Queue: Bill Budge > > Reviewed-by: Andreas Haas > > Reviewed-by: Georg Neis > > Cr-Commit-Position: refs/heads/master@{#72195} > > Bug: v8:9198 > Change-Id: I91e02b823af8ec925dacf075388fb22e3eeb3384 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2640890 > Reviewed-by: Bill Budge > Commit-Queue: Bill Budge > Cr-Commit-Position: refs/heads/master@{#72209} Bug: v8:9198 Change-Id: Ia5cf63af4e5991bc7cf42da9972ffd044fc829f0 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2733177 Commit-Queue: Bill Budge Reviewed-by: Andreas Haas Cr-Commit-Position: refs/heads/master@{#73238} --- BUILD.gn | 2 + src/codegen/aligned-slot-allocator.cc | 125 +++++++++ src/codegen/aligned-slot-allocator.h | 71 +++++ .../backend/arm/instruction-selector-arm.cc | 2 +- .../arm64/instruction-selector-arm64.cc | 2 +- .../backend/ia32/instruction-selector-ia32.cc | 2 +- .../backend/ppc/instruction-selector-ppc.cc | 2 +- src/compiler/backend/register-allocator.cc | 6 +- .../backend/s390/instruction-selector-s390.cc | 2 +- .../backend/x64/instruction-selector-x64.cc | 2 +- src/compiler/frame.cc | 30 ++- src/compiler/frame.h | 93 ++++--- src/compiler/wasm-compiler.cc | 80 +++--- src/wasm/wasm-linkage.h | 114 ++++----- test/unittests/BUILD.gn | 2 + .../aligned-slot-allocator-unittest.cc | 175 +++++++++++++ test/unittests/compiler/frame-unittest.cc | 242 ++++++++++++++++++ test/unittests/wasm/wasm-compiler-unittest.cc | 44 +++- 18 files changed, 836 insertions(+), 160 deletions(-) create mode 100644 src/codegen/aligned-slot-allocator.cc create mode 100644 src/codegen/aligned-slot-allocator.h create mode 100644 test/unittests/codegen/aligned-slot-allocator-unittest.cc create mode 100644 test/unittests/compiler/frame-unittest.cc diff --git a/BUILD.gn b/BUILD.gn index 74a9995ed4..e80f9dd96f 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -2261,6 +2261,7 @@ v8_header_set("v8_internal_headers") { "src/builtins/builtins.h", "src/builtins/constants-table-builder.h", "src/builtins/profile-data-reader.h", + "src/codegen/aligned-slot-allocator.h", "src/codegen/assembler-arch.h", "src/codegen/assembler-inl.h", "src/codegen/assembler.h", @@ -3534,6 +3535,7 @@ v8_source_set("v8_base_without_compiler") { "src/builtins/builtins-weak-refs.cc", "src/builtins/builtins.cc", "src/builtins/constants-table-builder.cc", + "src/codegen/aligned-slot-allocator.cc", "src/codegen/assembler.cc", "src/codegen/bailout-reason.cc", "src/codegen/code-comments.cc", diff --git a/src/codegen/aligned-slot-allocator.cc b/src/codegen/aligned-slot-allocator.cc new file mode 100644 index 0000000000..9e7ab09c81 --- /dev/null +++ b/src/codegen/aligned-slot-allocator.cc @@ -0,0 +1,125 @@ +// Copyright 2020 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. + +#include "src/codegen/aligned-slot-allocator.h" + +#include "src/base/bits.h" +#include "src/base/logging.h" + +namespace v8 { +namespace internal { + +int AlignedSlotAllocator::NextSlot(int n) const { + DCHECK(n == 1 || n == 2 || n == 4); + if (n <= 1 && IsValid(next1_)) return next1_; + if (n <= 2 && IsValid(next2_)) return next2_; + DCHECK(IsValid(next4_)); + return next4_; +} + +int AlignedSlotAllocator::Allocate(int n) { + DCHECK(n == 1 || n == 2 || n == 4); + // Check invariants. + DCHECK_EQ(0, next4_ & 3); + DCHECK_IMPLIES(IsValid(next2_), (next2_ & 1) == 0); + + // The sentinel value kInvalidSlot is used to indicate no slot. + // next1_ is the index of the 1 slot fragment, or kInvalidSlot. + // next2_ is the 2-aligned index of the 2 slot fragment, or kInvalidSlot. + // next4_ is the 4-aligned index of the next 4 slot group. It is always valid. + // In order to ensure we only have a single 1- or 2-slot fragment, we greedily + // use any fragment that satisfies the request. + int result = kInvalidSlot; + switch (n) { + case 1: { + if (IsValid(next1_)) { + result = next1_; + next1_ = kInvalidSlot; + } else if (IsValid(next2_)) { + result = next2_; + next1_ = result + 1; + next2_ = kInvalidSlot; + } else { + result = next4_; + next1_ = result + 1; + next2_ = result + 2; + next4_ += 4; + } + break; + } + case 2: { + if (IsValid(next2_)) { + result = next2_; + next2_ = kInvalidSlot; + } else { + result = next4_; + next2_ = result + 2; + next4_ += 4; + } + break; + } + case 4: { + result = next4_; + next4_ += 4; + break; + } + default: + UNREACHABLE(); + break; + } + DCHECK(IsValid(result)); + size_ = std::max(size_, result + n); + return result; +} + +int AlignedSlotAllocator::AllocateUnaligned(int n) { + DCHECK_GE(n, 0); + // Check invariants. + DCHECK_EQ(0, next4_ & 3); + DCHECK_IMPLIES(IsValid(next2_), (next2_ & 1) == 0); + + // Reserve |n| slots at |size_|, invalidate fragments below the new |size_|, + // and add any new fragments beyond the new |size_|. + int result = size_; + size_ += n; + switch (size_ & 3) { + case 0: { + next1_ = next2_ = kInvalidSlot; + next4_ = size_; + break; + } + case 1: { + next1_ = size_; + next2_ = size_ + 1; + next4_ = size_ + 3; + break; + } + case 2: { + next1_ = kInvalidSlot; + next2_ = size_; + next4_ = size_ + 2; + break; + } + case 3: { + next1_ = size_; + next2_ = kInvalidSlot; + next4_ = size_ + 1; + break; + } + } + return result; +} + +int AlignedSlotAllocator::Align(int n) { + DCHECK(base::bits::IsPowerOfTwo(n)); + DCHECK_LE(n, 4); + int mask = n - 1; + int misalignment = size_ & mask; + int padding = (n - misalignment) & mask; + AllocateUnaligned(padding); + return padding; +} + +} // namespace internal +} // namespace v8 diff --git a/src/codegen/aligned-slot-allocator.h b/src/codegen/aligned-slot-allocator.h new file mode 100644 index 0000000000..1abb711713 --- /dev/null +++ b/src/codegen/aligned-slot-allocator.h @@ -0,0 +1,71 @@ +// Copyright 2020 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. + +#ifndef V8_CODEGEN_ALIGNED_SLOT_ALLOCATOR_H_ +#define V8_CODEGEN_ALIGNED_SLOT_ALLOCATOR_H_ + +#include "src/base/macros.h" +#include "src/base/platform/platform.h" +#include "src/common/globals.h" + +namespace v8 { +namespace internal { + +// An aligned slot allocator. Allocates groups of 1, 2, or 4 slots such that the +// first slot of the group is aligned to the group size. The allocator can also +// allocate unaligned groups of arbitrary size, and an align the number of slots +// to 1, 2, or 4 slots. The allocator tries to be as thrifty as possible by +// reusing alignment padding slots in subsequent smaller slot allocations. +class V8_EXPORT_PRIVATE AlignedSlotAllocator { + public: + // Slots are always multiples of pointer-sized units. + static constexpr int kSlotSize = kSystemPointerSize; + + static int NumSlotsForWidth(int bytes) { + DCHECK_GT(bytes, 0); + return (bytes + kSlotSize - 1) / kSlotSize; + } + + AlignedSlotAllocator() = default; + + // Allocates |n| slots, where |n| must be 1, 2, or 4. Padding slots may be + // inserted for alignment. + // Returns the starting index of the slots, which is evenly divisible by |n|. + int Allocate(int n); + + // Gets the starting index of the slots that would be returned by Allocate(n). + int NextSlot(int n) const; + + // Allocates the given number of slots at the current end of the slot area, + // and returns the starting index of the slots. This resets any fragment + // slots, so subsequent allocations will be after the end of this one. + // AllocateUnaligned(0) can be used to partition the slot area, for example + // to make sure tagged values follow untagged values on a Frame. + int AllocateUnaligned(int n); + + // Aligns the slot area so that future allocations begin at the alignment. + // Returns the number of slots needed to align the slot area. + int Align(int n); + + // Returns the size of the slot area, in slots. This will be greater than any + // already allocated slot index. + int Size() const { return size_; } + + private: + static constexpr int kInvalidSlot = -1; + + static bool IsValid(int slot) { return slot > kInvalidSlot; } + + int next1_ = kInvalidSlot; + int next2_ = kInvalidSlot; + int next4_ = 0; + int size_ = 0; + + DISALLOW_NEW_AND_DELETE() +}; + +} // namespace internal +} // namespace v8 + +#endif // V8_CODEGEN_ALIGNED_SLOT_ALLOCATOR_H_ diff --git a/src/compiler/backend/arm/instruction-selector-arm.cc b/src/compiler/backend/arm/instruction-selector-arm.cc index 7cce5e3db5..ec0d5c76b0 100644 --- a/src/compiler/backend/arm/instruction-selector-arm.cc +++ b/src/compiler/backend/arm/instruction-selector-arm.cc @@ -490,7 +490,7 @@ void VisitPairAtomicBinOp(InstructionSelector* selector, Node* node, void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/backend/arm64/instruction-selector-arm64.cc b/src/compiler/backend/arm64/instruction-selector-arm64.cc index 1f7bdcab6b..9effd783de 100644 --- a/src/compiler/backend/arm64/instruction-selector-arm64.cc +++ b/src/compiler/backend/arm64/instruction-selector-arm64.cc @@ -559,7 +559,7 @@ int32_t LeftShiftForReducedMultiply(Matcher* m) { void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/backend/ia32/instruction-selector-ia32.cc b/src/compiler/backend/ia32/instruction-selector-ia32.cc index 84d7d20230..4bf436b708 100644 --- a/src/compiler/backend/ia32/instruction-selector-ia32.cc +++ b/src/compiler/backend/ia32/instruction-selector-ia32.cc @@ -398,7 +398,7 @@ void VisitRROI8x16SimdShift(InstructionSelector* selector, Node* node, void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/backend/ppc/instruction-selector-ppc.cc b/src/compiler/backend/ppc/instruction-selector-ppc.cc index eb7972af9e..72dae6bcfd 100644 --- a/src/compiler/backend/ppc/instruction-selector-ppc.cc +++ b/src/compiler/backend/ppc/instruction-selector-ppc.cc @@ -155,7 +155,7 @@ void VisitBinop(InstructionSelector* selector, Node* node, void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/backend/register-allocator.cc b/src/compiler/backend/register-allocator.cc index 84145c8779..71e121661d 100644 --- a/src/compiler/backend/register-allocator.cc +++ b/src/compiler/backend/register-allocator.cc @@ -4519,9 +4519,11 @@ void OperandAssigner::AssignSpillSlots() { for (SpillRange* range : spill_ranges) { data()->tick_counter()->TickAndMaybeEnterSafepoint(); if (range == nullptr || range->IsEmpty()) continue; - // Allocate a new operand referring to the spill slot. if (!range->HasSlot()) { - int index = data()->frame()->AllocateSpillSlot(range->byte_width()); + // Allocate a new operand referring to the spill slot, aligned to the + // operand size. + int width = range->byte_width(); + int index = data()->frame()->AllocateSpillSlot(width, width); range->set_assigned_slot(index); } } diff --git a/src/compiler/backend/s390/instruction-selector-s390.cc b/src/compiler/backend/s390/instruction-selector-s390.cc index cacb15bea6..143ba59f77 100644 --- a/src/compiler/backend/s390/instruction-selector-s390.cc +++ b/src/compiler/backend/s390/instruction-selector-s390.cc @@ -680,7 +680,7 @@ void VisitBinOp(InstructionSelector* selector, Node* node, void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/backend/x64/instruction-selector-x64.cc b/src/compiler/backend/x64/instruction-selector-x64.cc index 0ab7ec8485..7589c5a1dc 100644 --- a/src/compiler/backend/x64/instruction-selector-x64.cc +++ b/src/compiler/backend/x64/instruction-selector-x64.cc @@ -338,7 +338,7 @@ ArchOpcode GetStoreOpcode(StoreRepresentation store_rep) { void InstructionSelector::VisitStackSlot(Node* node) { StackSlotRepresentation rep = StackSlotRepresentationOf(node->op()); - int slot = frame_->AllocateSpillSlot(rep.size()); + int slot = frame_->AllocateSpillSlot(rep.size(), rep.alignment()); OperandGenerator g(this); Emit(kArchStackSlot, g.DefineAsRegister(node), diff --git a/src/compiler/frame.cc b/src/compiler/frame.cc index 98938e8358..b33e9e1354 100644 --- a/src/compiler/frame.cc +++ b/src/compiler/frame.cc @@ -12,27 +12,29 @@ namespace compiler { Frame::Frame(int fixed_frame_size_in_slots) : fixed_slot_count_(fixed_frame_size_in_slots), - frame_slot_count_(fixed_frame_size_in_slots), - spill_slot_count_(0), - return_slot_count_(0), allocated_registers_(nullptr), - allocated_double_registers_(nullptr) {} + allocated_double_registers_(nullptr) { + slot_allocator_.AllocateUnaligned(fixed_frame_size_in_slots); +} void Frame::AlignFrame(int alignment) { - int alignment_slots = alignment / kSystemPointerSize; - // In the calculations below we assume that alignment_slots is a power of 2. - DCHECK(base::bits::IsPowerOfTwo(alignment_slots)); +#if DEBUG + spill_slots_finished_ = true; +#endif + // In the calculations below we assume that alignment is a power of 2. + DCHECK(base::bits::IsPowerOfTwo(alignment)); + int alignment_in_slots = AlignedSlotAllocator::NumSlotsForWidth(alignment); // We have to align return slots separately, because they are claimed // separately on the stack. - int return_delta = - alignment_slots - (return_slot_count_ & (alignment_slots - 1)); - if (return_delta != alignment_slots) { - frame_slot_count_ += return_delta; + const int mask = alignment_in_slots - 1; + int return_delta = alignment_in_slots - (return_slot_count_ & mask); + if (return_delta != alignment_in_slots) { + return_slot_count_ += return_delta; } - int delta = alignment_slots - (frame_slot_count_ & (alignment_slots - 1)); - if (delta != alignment_slots) { - frame_slot_count_ += delta; + int delta = alignment_in_slots - (slot_allocator_.Size() & mask); + if (delta != alignment_in_slots) { + slot_allocator_.Align(alignment_in_slots); if (spill_slot_count_ != 0) { spill_slot_count_ += delta; } diff --git a/src/compiler/frame.h b/src/compiler/frame.h index 7fc0c27b84..21f0f59171 100644 --- a/src/compiler/frame.h +++ b/src/compiler/frame.h @@ -5,6 +5,8 @@ #ifndef V8_COMPILER_FRAME_H_ #define V8_COMPILER_FRAME_H_ +#include "src/base/bits.h" +#include "src/codegen/aligned-slot-allocator.h" #include "src/execution/frame-constants.h" #include "src/utils/bit-vector.h" @@ -92,7 +94,9 @@ class V8_EXPORT_PRIVATE Frame : public ZoneObject { Frame(const Frame&) = delete; Frame& operator=(const Frame&) = delete; - inline int GetTotalFrameSlotCount() const { return frame_slot_count_; } + inline int GetTotalFrameSlotCount() const { + return slot_allocator_.Size() + return_slot_count_; + } inline int GetFixedSlotCount() const { return fixed_slot_count_; } inline int GetSpillSlotCount() const { return spill_slot_count_; } inline int GetReturnSlotCount() const { return return_slot_count_; } @@ -112,38 +116,55 @@ class V8_EXPORT_PRIVATE Frame : public ZoneObject { } void AlignSavedCalleeRegisterSlots(int alignment = kDoubleSize) { - int alignment_slots = alignment / kSystemPointerSize; - int delta = alignment_slots - (frame_slot_count_ & (alignment_slots - 1)); - if (delta != alignment_slots) { - frame_slot_count_ += delta; - } - spill_slot_count_ += delta; +#if DEBUG + spill_slots_finished_ = true; +#endif + DCHECK(base::bits::IsPowerOfTwo(alignment)); + DCHECK_LE(alignment, kSimd128Size); + int alignment_in_slots = AlignedSlotAllocator::NumSlotsForWidth(alignment); + int padding = slot_allocator_.Align(alignment_in_slots); + spill_slot_count_ += padding; } void AllocateSavedCalleeRegisterSlots(int count) { - frame_slot_count_ += count; +#if DEBUG + spill_slots_finished_ = true; +#endif + slot_allocator_.AllocateUnaligned(count); } int AllocateSpillSlot(int width, int alignment = 0) { - DCHECK_EQ(frame_slot_count_, + DCHECK_EQ(GetTotalFrameSlotCount(), fixed_slot_count_ + spill_slot_count_ + return_slot_count_); - int frame_slot_count_before = frame_slot_count_; - if (alignment > kSystemPointerSize) { - // Slots are pointer sized, so alignment greater than a pointer size - // requires allocating additional slots. - width += alignment - kSystemPointerSize; + // Never allocate spill slots after the callee-saved slots are defined. + DCHECK(!spill_slots_finished_); + int actual_width = std::max({width, AlignedSlotAllocator::kSlotSize}); + int actual_alignment = + std::max({alignment, AlignedSlotAllocator::kSlotSize}); + int slots = AlignedSlotAllocator::NumSlotsForWidth(actual_width); + int old_end = slot_allocator_.Size(); + int slot; + if (actual_width == actual_alignment) { + // Simple allocation, alignment equal to width. + slot = slot_allocator_.Allocate(slots); + } else { + // Complex allocation, alignment different from width. + if (actual_alignment > AlignedSlotAllocator::kSlotSize) { + // Alignment required. + int alignment_in_slots = + AlignedSlotAllocator::NumSlotsForWidth(actual_alignment); + slot_allocator_.Align(alignment_in_slots); + } + slot = slot_allocator_.AllocateUnaligned(slots); } - AllocateAlignedFrameSlots(width); - spill_slot_count_ += frame_slot_count_ - frame_slot_count_before; - return frame_slot_count_ - return_slot_count_ - 1; + int end = slot_allocator_.Size(); + + spill_slot_count_ += end - old_end; + return slot + slots - 1; } void EnsureReturnSlots(int count) { - if (count > return_slot_count_) { - count -= return_slot_count_; - frame_slot_count_ += count; - return_slot_count_ += count; - } + return_slot_count_ = std::max(return_slot_count_, count); } void AlignFrame(int alignment = kDoubleSize); @@ -151,30 +172,22 @@ class V8_EXPORT_PRIVATE Frame : public ZoneObject { int ReserveSpillSlots(size_t slot_count) { DCHECK_EQ(0, spill_slot_count_); spill_slot_count_ += static_cast(slot_count); - frame_slot_count_ += static_cast(slot_count); - return frame_slot_count_ - 1; - } - - private: - void AllocateAlignedFrameSlots(int width) { - DCHECK_LT(0, width); - int new_frame_slots = (width + kSystemPointerSize - 1) / kSystemPointerSize; - // Align to 8 bytes if width is a multiple of 8 bytes, and to 16 bytes if - // multiple of 16. - int align_to = - (width & 15) == 0 ? 16 : (width & 7) == 0 ? 8 : kSystemPointerSize; - frame_slot_count_ = RoundUp(frame_slot_count_ + new_frame_slots, - align_to / kSystemPointerSize); - DCHECK_LT(0, frame_slot_count_); + slot_allocator_.AllocateUnaligned(static_cast(slot_count)); + return slot_allocator_.Size() - 1; } private: int fixed_slot_count_; - int frame_slot_count_; - int spill_slot_count_; - int return_slot_count_; + int spill_slot_count_ = 0; + // Account for return slots separately. Conceptually, they follow all + // allocated spill slots. + int return_slot_count_ = 0; + AlignedSlotAllocator slot_allocator_; BitVector* allocated_registers_; BitVector* allocated_double_registers_; +#if DEBUG + bool spill_slots_finished_ = false; +#endif }; // Represents an offset from either the stack pointer or frame pointer. diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc index 2eb458d421..bfd151e223 100644 --- a/src/compiler/wasm-compiler.cc +++ b/src/compiler/wasm-compiler.cc @@ -7956,8 +7956,9 @@ class LinkageLocationAllocator { public: template constexpr LinkageLocationAllocator(const Register (&gp)[kNumGpRegs], - const DoubleRegister (&fp)[kNumFpRegs]) - : allocator_(wasm::LinkageAllocator(gp, fp)) {} + const DoubleRegister (&fp)[kNumFpRegs], + int slot_offset) + : allocator_(wasm::LinkageAllocator(gp, fp)), slot_offset_(slot_offset) {} LinkageLocation Next(MachineRepresentation rep) { MachineType type = MachineType::TypeForRepresentation(rep); @@ -7971,15 +7972,19 @@ class LinkageLocationAllocator { return LinkageLocation::ForRegister(reg_code, type); } // Cannot use register; use stack slot. - int index = -1 - allocator_.NextStackSlot(rep); + int index = -1 - (slot_offset_ + allocator_.NextStackSlot(rep)); return LinkageLocation::ForCallerFrameSlot(index, type); } - void SetStackOffset(int offset) { allocator_.SetStackOffset(offset); } int NumStackSlots() const { return allocator_.NumStackSlots(); } + void EndSlotArea() { allocator_.EndSlotArea(); } private: wasm::LinkageAllocator allocator_; + // Since params and returns are in different stack frames, we must allocate + // them separately. Parameter slots don't need an offset, but return slots + // must be offset to just before the param slots, using this |slot_offset_|. + int slot_offset_; }; } // namespace @@ -7997,8 +8002,8 @@ CallDescriptor* GetWasmCallDescriptor( fsig->parameter_count() + extra_params); // Add register and/or stack parameter(s). - LinkageLocationAllocator params(wasm::kGpParamRegisters, - wasm::kFpParamRegisters); + LinkageLocationAllocator params( + wasm::kGpParamRegisters, wasm::kFpParamRegisters, 0 /* no slot offset */); // The instance object. locations.AddParam(params.Next(MachineRepresentation::kTaggedPointer)); @@ -8015,6 +8020,10 @@ CallDescriptor* GetWasmCallDescriptor( auto l = params.Next(param); locations.AddParamAt(i + param_offset, l); } + + // End the untagged area, so tagged slots come after. + params.EndSlotArea(); + for (size_t i = 0; i < parameter_count; i++) { MachineRepresentation param = fsig->GetParam(i).machine_representation(); // Skip untagged parameters. @@ -8030,22 +8039,21 @@ CallDescriptor* GetWasmCallDescriptor( kJSFunctionRegister.code(), MachineType::TaggedPointer())); } - // Add return location(s). - LinkageLocationAllocator rets(wasm::kGpReturnRegisters, - wasm::kFpReturnRegisters); - int parameter_slots = params.NumStackSlots(); if (ShouldPadArguments(parameter_slots)) parameter_slots++; - rets.SetStackOffset(parameter_slots); + // Add return location(s). + LinkageLocationAllocator rets(wasm::kGpReturnRegisters, + wasm::kFpReturnRegisters, parameter_slots); const int return_count = static_cast(locations.return_count_); for (int i = 0; i < return_count; i++) { MachineRepresentation ret = fsig->GetReturn(i).machine_representation(); - auto l = rets.Next(ret); - locations.AddReturn(l); + locations.AddReturn(rets.Next(ret)); } + int return_slots = rets.NumStackSlots(); + const RegList kCalleeSaveRegisters = 0; const RegList kCalleeSaveFPRegisters = 0; @@ -8072,7 +8080,7 @@ CallDescriptor* GetWasmCallDescriptor( target_type, // target MachineType target_loc, // target location locations.Build(), // location_sig - parameter_slots, // stack_parameter_count + parameter_slots, // parameter slot count compiler::Operator::kNoProperties, // properties kCalleeSaveRegisters, // callee-saved registers kCalleeSaveFPRegisters, // callee-saved fp regs @@ -8080,7 +8088,7 @@ CallDescriptor* GetWasmCallDescriptor( "wasm-call", // debug name StackArgumentOrder::kDefault, // order of the arguments in the stack 0, // allocatable registers - rets.NumStackSlots() - parameter_slots); // stack_return_count + return_slots); // return slot count } namespace { @@ -8113,8 +8121,9 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith( (call_descriptor->GetInputLocation(call_descriptor->InputCount() - 1) == LinkageLocation::ForRegister(kJSFunctionRegister.code(), MachineType::TaggedPointer())); - LinkageLocationAllocator params(wasm::kGpParamRegisters, - wasm::kFpParamRegisters); + LinkageLocationAllocator params( + wasm::kGpParamRegisters, wasm::kFpParamRegisters, 0 /* no slot offset */); + for (size_t i = 0, e = call_descriptor->ParameterCount() - (has_callable_param ? 1 : 0); i < e; i++) { @@ -8132,9 +8141,12 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith( kJSFunctionRegister.code(), MachineType::TaggedPointer())); } + int parameter_slots = params.NumStackSlots(); + if (ShouldPadArguments(parameter_slots)) parameter_slots++; + LinkageLocationAllocator rets(wasm::kGpReturnRegisters, - wasm::kFpReturnRegisters); - rets.SetStackOffset(params.NumStackSlots()); + wasm::kFpReturnRegisters, parameter_slots); + for (size_t i = 0; i < call_descriptor->ReturnCount(); i++) { if (call_descriptor->GetReturnType(i) == input_type) { for (size_t j = 0; j < num_replacements; j++) { @@ -8146,20 +8158,22 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith( } } - return zone->New( // -- - call_descriptor->kind(), // kind - call_descriptor->GetInputType(0), // target MachineType - call_descriptor->GetInputLocation(0), // target location - locations.Build(), // location_sig - params.NumStackSlots(), // stack_parameter_count - call_descriptor->properties(), // properties - call_descriptor->CalleeSavedRegisters(), // callee-saved registers - call_descriptor->CalleeSavedFPRegisters(), // callee-saved fp regs - call_descriptor->flags(), // flags - call_descriptor->debug_name(), // debug name - call_descriptor->GetStackArgumentOrder(), // stack order - call_descriptor->AllocatableRegisters(), // allocatable registers - rets.NumStackSlots() - params.NumStackSlots()); // stack_return_count + int return_slots = rets.NumStackSlots(); + + return zone->New( // -- + call_descriptor->kind(), // kind + call_descriptor->GetInputType(0), // target MachineType + call_descriptor->GetInputLocation(0), // target location + locations.Build(), // location_sig + parameter_slots, // parameter slot count + call_descriptor->properties(), // properties + call_descriptor->CalleeSavedRegisters(), // callee-saved registers + call_descriptor->CalleeSavedFPRegisters(), // callee-saved fp regs + call_descriptor->flags(), // flags + call_descriptor->debug_name(), // debug name + call_descriptor->GetStackArgumentOrder(), // stack order + call_descriptor->AllocatableRegisters(), // allocatable registers + return_slots); // return slot count } } // namespace diff --git a/src/wasm/wasm-linkage.h b/src/wasm/wasm-linkage.h index fd27d7108d..683d43d88a 100644 --- a/src/wasm/wasm-linkage.h +++ b/src/wasm/wasm-linkage.h @@ -5,6 +5,7 @@ #ifndef V8_WASM_WASM_LINKAGE_H_ #define V8_WASM_WASM_LINKAGE_H_ +#include "src/codegen/aligned-slot-allocator.h" #include "src/codegen/assembler-arch.h" #include "src/codegen/machine-type.h" #include "src/codegen/signature.h" @@ -44,7 +45,7 @@ constexpr DoubleRegister kFpReturnRegisters[] = {xmm1, xmm2}; // =========================================================================== constexpr Register kGpParamRegisters[] = {r3, r0, r2, r6}; constexpr Register kGpReturnRegisters[] = {r0, r1}; -// ARM d-registers must be in ascending order for correct allocation. +// ARM d-registers must be in even/odd D-register pairs for correct allocation. constexpr DoubleRegister kFpParamRegisters[] = {d0, d1, d2, d3, d4, d5, d6, d7}; constexpr DoubleRegister kFpReturnRegisters[] = {d0, d1}; @@ -145,18 +146,27 @@ class LinkageAllocator { bool CanAllocateFP(MachineRepresentation rep) const { #if V8_TARGET_ARCH_ARM switch (rep) { - case MachineRepresentation::kFloat32: - return fp_offset_ < fp_count_ && fp_regs_[fp_offset_].code() < 16; - case MachineRepresentation::kFloat64: - return extra_double_reg_ >= 0 || fp_offset_ < fp_count_; - case MachineRepresentation::kSimd128: - return ((fp_offset_ + 1) & ~1) + 1 < fp_count_; + case MachineRepresentation::kFloat32: { + // Get the next D-register (Liftoff only uses the even S-registers). + int next = fp_allocator_.NextSlot(2) / 2; + // Only the lower 16 D-registers alias S-registers. + return next < fp_count_ && fp_regs_[next].code() < 16; + } + case MachineRepresentation::kFloat64: { + int next = fp_allocator_.NextSlot(2) / 2; + return next < fp_count_; + } + case MachineRepresentation::kSimd128: { + int next = fp_allocator_.NextSlot(4) / 2; + return next < fp_count_ - 1; // 2 D-registers are required. + } default: UNREACHABLE(); return false; } -#endif +#else return fp_offset_ < fp_count_; +#endif } int NextGpReg() { @@ -165,80 +175,58 @@ class LinkageAllocator { } int NextFpReg(MachineRepresentation rep) { + DCHECK(CanAllocateFP(rep)); #if V8_TARGET_ARCH_ARM switch (rep) { case MachineRepresentation::kFloat32: { - // Liftoff uses only even-numbered f32 registers, and encodes them using - // the code of the corresponding f64 register. This limits the calling - // interface to only using the even-numbered f32 registers. + // Liftoff uses only even-numbered S-registers, and encodes them using + // the code of the corresponding D-register. This limits the calling + // interface to only using the even-numbered S-registers. int d_reg_code = NextFpReg(MachineRepresentation::kFloat64); - DCHECK_GT(16, d_reg_code); // D-registers 16 - 31 can't split. + DCHECK_GT(16, d_reg_code); // D16 - D31 don't alias S-registers. return d_reg_code * 2; } case MachineRepresentation::kFloat64: { - // Use the extra D-register if there is one. - if (extra_double_reg_ >= 0) { - int reg_code = extra_double_reg_; - extra_double_reg_ = -1; - return reg_code; - } - DCHECK_LT(fp_offset_, fp_count_); - return fp_regs_[fp_offset_++].code(); + int next = fp_allocator_.Allocate(2) / 2; + return fp_regs_[next].code(); } case MachineRepresentation::kSimd128: { - // Q-register must be an even-odd pair, so we must try to allocate at - // the end, not using extra_double_reg_. If we are at an odd D-register, - // skip past it (saving it to extra_double_reg_). - DCHECK_LT(((fp_offset_ + 1) & ~1) + 1, fp_count_); - int d_reg1_code = fp_regs_[fp_offset_++].code(); - if (d_reg1_code % 2 != 0) { - // If we're misaligned then extra_double_reg_ must have been consumed. - DCHECK_EQ(-1, extra_double_reg_); - int odd_double_reg = d_reg1_code; - d_reg1_code = fp_regs_[fp_offset_++].code(); - extra_double_reg_ = odd_double_reg; - } - // Combine the current D-register with the next to form a Q-register. - int d_reg2_code = fp_regs_[fp_offset_++].code(); - DCHECK_EQ(0, d_reg1_code % 2); - DCHECK_EQ(d_reg1_code + 1, d_reg2_code); - USE(d_reg2_code); - return d_reg1_code / 2; + int next = fp_allocator_.Allocate(4) / 2; + int d_reg_code = fp_regs_[next].code(); + // Check that result and the next D-register pair. + DCHECK_EQ(0, d_reg_code % 2); + DCHECK_EQ(d_reg_code + 1, fp_regs_[next + 1].code()); + return d_reg_code / 2; } default: UNREACHABLE(); } #else - DCHECK_LT(fp_offset_, fp_count_); return fp_regs_[fp_offset_++].code(); #endif } // Stackslots are counted upwards starting from 0 (or the offset set by - // {SetStackOffset}. - int NumStackSlots(MachineRepresentation type) { - return std::max(1, ElementSizeInBytes(type) / kSystemPointerSize); - } - - // Stackslots are counted upwards starting from 0 (or the offset set by - // {SetStackOffset}. If {type} needs more than - // one stack slot, the lowest used stack slot is returned. + // {SetStackOffset}. If {type} needs more than one stack slot, the lowest + // used stack slot is returned. int NextStackSlot(MachineRepresentation type) { - int num_stack_slots = NumStackSlots(type); - int offset = stack_offset_; - stack_offset_ += num_stack_slots; - return offset; + int num_slots = + AlignedSlotAllocator::NumSlotsForWidth(ElementSizeInBytes(type)); + int slot = slot_allocator_.Allocate(num_slots); + return slot; } // Set an offset for the stack slots returned by {NextStackSlot} and // {NumStackSlots}. Can only be called before any call to {NextStackSlot}. - void SetStackOffset(int num) { - DCHECK_LE(0, num); - DCHECK_EQ(0, stack_offset_); - stack_offset_ = num; + void SetStackOffset(int offset) { + DCHECK_LE(0, offset); + DCHECK_EQ(0, slot_allocator_.Size()); + slot_allocator_.AllocateUnaligned(offset); } - int NumStackSlots() const { return stack_offset_; } + int NumStackSlots() const { return slot_allocator_.Size(); } + + void EndSlotArea() { slot_allocator_.AllocateUnaligned(0); } private: const int gp_count_; @@ -246,16 +234,16 @@ class LinkageAllocator { const Register* const gp_regs_; const int fp_count_; +#if V8_TARGET_ARCH_ARM + // Use an aligned slot allocator to model ARM FP register aliasing. The slots + // are 32 bits, so 2 slots are required for a D-register, 4 for a Q-register. + AlignedSlotAllocator fp_allocator_; +#else int fp_offset_ = 0; +#endif const DoubleRegister* const fp_regs_; -#if V8_TARGET_ARCH_ARM - // Track fragments of registers below fp_offset_ here. There can only be one - // extra double register. - int extra_double_reg_ = -1; -#endif - - int stack_offset_ = 0; + AlignedSlotAllocator slot_allocator_; }; } // namespace wasm diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index 65a2c0af82..53e68a03be 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -226,6 +226,7 @@ v8_source_set("unittests_sources") { "base/utils/random-number-generator-unittest.cc", "base/vlq-base64-unittest.cc", "base/vlq-unittest.cc", + "codegen/aligned-slot-allocator-unittest.cc", "codegen/code-stub-assembler-unittest.cc", "codegen/code-stub-assembler-unittest.h", "codegen/register-configuration-unittest.cc", @@ -250,6 +251,7 @@ v8_source_set("unittests_sources") { "compiler/decompression-optimizer-unittest.cc", "compiler/diamond-unittest.cc", "compiler/effect-control-linearizer-unittest.cc", + "compiler/frame-unittest.cc", "compiler/graph-reducer-unittest.cc", "compiler/graph-reducer-unittest.h", "compiler/graph-trimmer-unittest.cc", diff --git a/test/unittests/codegen/aligned-slot-allocator-unittest.cc b/test/unittests/codegen/aligned-slot-allocator-unittest.cc new file mode 100644 index 0000000000..3b04f7888a --- /dev/null +++ b/test/unittests/codegen/aligned-slot-allocator-unittest.cc @@ -0,0 +1,175 @@ +// Copyright 2020 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. + +#include "src/codegen/aligned-slot-allocator.h" + +#include "src/base/bits.h" +#include "testing/gtest-support.h" + +namespace v8 { +namespace internal { + +class AlignedSlotAllocatorUnitTest : public ::testing::Test { + public: + AlignedSlotAllocatorUnitTest() = default; + ~AlignedSlotAllocatorUnitTest() override = default; + + // Helper method to test AlignedSlotAllocator::Allocate. + void Allocate(int size, int expected) { + int next = allocator_.NextSlot(size); + int result = allocator_.Allocate(size); + EXPECT_EQ(next, result); // NextSlot/Allocate are consistent. + EXPECT_EQ(expected, result); + EXPECT_EQ(0, result & (size - 1)); // result is aligned to size. + int slot_end = result + static_cast(base::bits::RoundUpToPowerOfTwo32( + static_cast(size))); + EXPECT_LE(slot_end, allocator_.Size()); // allocator Size is beyond slot. + } + + // Helper method to test AlignedSlotAllocator::AllocateUnaligned. + void AllocateUnaligned(int size, int expected, int expected1, int expected2, + int expected4) { + int size_before = allocator_.Size(); + int result = allocator_.AllocateUnaligned(size); + EXPECT_EQ(size_before, result); // AllocateUnaligned/Size are consistent. + EXPECT_EQ(expected, result); + EXPECT_EQ(result + size, allocator_.Size()); + EXPECT_EQ(expected1, allocator_.NextSlot(1)); + EXPECT_EQ(expected2, allocator_.NextSlot(2)); + EXPECT_EQ(expected4, allocator_.NextSlot(4)); + } + + AlignedSlotAllocator allocator_; +}; + +TEST_F(AlignedSlotAllocatorUnitTest, NumSlotsForWidth) { + constexpr int kSlotBytes = AlignedSlotAllocator::kSlotSize; + for (int slot_size = 1; slot_size <= 4 * kSlotBytes; ++slot_size) { + EXPECT_EQ(AlignedSlotAllocator::NumSlotsForWidth(slot_size), + (slot_size + kSlotBytes - 1) / kSlotBytes); + } +} + +TEST_F(AlignedSlotAllocatorUnitTest, Allocate1) { + Allocate(1, 0); + EXPECT_EQ(2, allocator_.NextSlot(2)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + Allocate(1, 1); + EXPECT_EQ(2, allocator_.NextSlot(2)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + Allocate(1, 2); + EXPECT_EQ(4, allocator_.NextSlot(2)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + Allocate(1, 3); + EXPECT_EQ(4, allocator_.NextSlot(2)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + // Make sure we use 1-fragments. + Allocate(1, 4); + Allocate(2, 6); + Allocate(1, 5); + + // Make sure we use 2-fragments. + Allocate(2, 8); + Allocate(1, 10); + Allocate(1, 11); +} + +TEST_F(AlignedSlotAllocatorUnitTest, Allocate2) { + Allocate(2, 0); + EXPECT_EQ(2, allocator_.NextSlot(1)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + Allocate(2, 2); + EXPECT_EQ(4, allocator_.NextSlot(1)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + + // Make sure we use 2-fragments. + Allocate(1, 4); + Allocate(2, 6); + Allocate(2, 8); +} + +TEST_F(AlignedSlotAllocatorUnitTest, Allocate4) { + Allocate(4, 0); + EXPECT_EQ(4, allocator_.NextSlot(1)); + EXPECT_EQ(4, allocator_.NextSlot(2)); + + Allocate(1, 4); + Allocate(4, 8); + + Allocate(2, 6); + Allocate(4, 12); +} + +TEST_F(AlignedSlotAllocatorUnitTest, AllocateUnaligned) { + AllocateUnaligned(1, 0, 1, 2, 4); + AllocateUnaligned(1, 1, 2, 2, 4); + + Allocate(1, 2); + + AllocateUnaligned(2, 3, 5, 6, 8); + + // Advance to leave 1- and 2- fragments below Size. + Allocate(4, 8); + + // AllocateUnaligned should allocate at the end, and clear fragments. + AllocateUnaligned(0, 12, 12, 12, 12); +} + +TEST_F(AlignedSlotAllocatorUnitTest, LargeAllocateUnaligned) { + AllocateUnaligned(11, 0, 11, 12, 12); + AllocateUnaligned(11, 11, 22, 22, 24); + AllocateUnaligned(13, 22, 35, 36, 36); +} + +TEST_F(AlignedSlotAllocatorUnitTest, Size) { + allocator_.Allocate(1); + EXPECT_EQ(1, allocator_.Size()); + // Allocate 2, leaving a fragment at 1. Size should be at 4. + allocator_.Allocate(2); + EXPECT_EQ(4, allocator_.Size()); + // Allocate should consume fragment. + EXPECT_EQ(1, allocator_.Allocate(1)); + // Size should still be 4. + EXPECT_EQ(4, allocator_.Size()); +} + +TEST_F(AlignedSlotAllocatorUnitTest, Align) { + EXPECT_EQ(0, allocator_.Align(1)); + EXPECT_EQ(0, allocator_.Size()); + + // Allocate 1 to become misaligned. + Allocate(1, 0); + + // 4-align. + EXPECT_EQ(3, allocator_.Align(4)); + EXPECT_EQ(4, allocator_.NextSlot(1)); + EXPECT_EQ(4, allocator_.NextSlot(2)); + EXPECT_EQ(4, allocator_.NextSlot(4)); + EXPECT_EQ(4, allocator_.Size()); + + // Allocate 2 to become misaligned. + Allocate(2, 4); + + // 4-align. + EXPECT_EQ(2, allocator_.Align(4)); + EXPECT_EQ(8, allocator_.NextSlot(1)); + EXPECT_EQ(8, allocator_.NextSlot(2)); + EXPECT_EQ(8, allocator_.NextSlot(4)); + EXPECT_EQ(8, allocator_.Size()); + + // No change when we're already aligned. + EXPECT_EQ(0, allocator_.Align(2)); + EXPECT_EQ(8, allocator_.NextSlot(1)); + EXPECT_EQ(8, allocator_.NextSlot(2)); + EXPECT_EQ(8, allocator_.NextSlot(4)); + EXPECT_EQ(8, allocator_.Size()); +} + +} // namespace internal +} // namespace v8 diff --git a/test/unittests/compiler/frame-unittest.cc b/test/unittests/compiler/frame-unittest.cc new file mode 100644 index 0000000000..f74e4d34ec --- /dev/null +++ b/test/unittests/compiler/frame-unittest.cc @@ -0,0 +1,242 @@ +// Copyright 2021 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. + +#include "src/compiler/frame.h" + +#include "src/codegen/aligned-slot-allocator.h" +#include "testing/gtest-support.h" + +namespace v8 { +namespace internal { +namespace compiler { + +namespace { +constexpr int kSlotSize = AlignedSlotAllocator::kSlotSize; + +constexpr int kFixed1 = 1; +constexpr int kFixed3 = 3; +} // namespace + +class FrameTest : public ::testing::Test { + public: + FrameTest() = default; + ~FrameTest() override = default; +}; + +TEST_F(FrameTest, Constructor) { + Frame frame(kFixed3); + EXPECT_EQ(kFixed3, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, ReserveSpillSlots) { + Frame frame(kFixed3); + constexpr int kReserve2 = 2; + + frame.ReserveSpillSlots(kReserve2); + EXPECT_EQ(kFixed3 + kReserve2, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(kReserve2, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, EnsureReturnSlots) { + Frame frame(kFixed3); + constexpr int kReturn3 = 3; + constexpr int kReturn5 = 5; + constexpr int kReturn2 = 2; + + frame.EnsureReturnSlots(kReturn3); + EXPECT_EQ(kFixed3 + kReturn3, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn3, frame.GetReturnSlotCount()); + + // Returns should grow by 2 slots. + frame.EnsureReturnSlots(kReturn5); + EXPECT_EQ(kFixed3 + kReturn5, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn5, frame.GetReturnSlotCount()); + + // Returns shouldn't grow. + frame.EnsureReturnSlots(kReturn2); + EXPECT_EQ(kFixed3 + kReturn5, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn5, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSavedCalleeRegisterSlots) { + Frame frame(kFixed3); + constexpr int kFirstSlots = 2; + constexpr int kSecondSlots = 3; + + frame.AllocateSavedCalleeRegisterSlots(kFirstSlots); + EXPECT_EQ(kFixed3 + kFirstSlots, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + frame.AllocateSavedCalleeRegisterSlots(kSecondSlots); + EXPECT_EQ(kFixed3 + kFirstSlots + kSecondSlots, + frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(0, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AlignSavedCalleeRegisterSlots) { + Frame frame(kFixed3); + constexpr int kSlots = 2; // An even number leaves the slots misaligned. + + frame.AllocateSavedCalleeRegisterSlots(kSlots); + + // Align, which should add 1 padding slot. + frame.AlignSavedCalleeRegisterSlots(2 * kSlotSize); + EXPECT_EQ(kFixed3 + kSlots + 1, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(1, frame.GetSpillSlotCount()); // padding + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + // Align again, which should not add a padding slot. + frame.AlignSavedCalleeRegisterSlots(2 * kSlotSize); + EXPECT_EQ(kFixed3 + kSlots + 1, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(1, frame.GetSpillSlotCount()); // padding + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSpillSlotAligned) { + Frame frame(kFixed1); + + // Allocate a quad slot, which must add 3 padding slots. Frame returns the + // last index of the 4 slot allocation. + int end = kFixed1 + 3 + 4; + int slot = kFixed1 + 3 + 4 - 1; + EXPECT_EQ(slot, frame.AllocateSpillSlot(4 * kSlotSize, 4 * kSlotSize)); + EXPECT_EQ(end, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(end - kFixed1, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + // Allocate a double slot, which should leave the first padding slot and + // take the last two slots of padding. + slot = kFixed1 + 1 + 2 - 1; + EXPECT_EQ(slot, frame.AllocateSpillSlot(2 * kSlotSize, 2 * kSlotSize)); + EXPECT_EQ(end, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(end - kFixed1, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + // Allocate a single slot, which should take the last padding slot. + slot = kFixed1; + EXPECT_EQ(slot, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + EXPECT_EQ(end, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(end - kFixed1, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSpillSlotAlignedWithReturns) { + Frame frame(kFixed3); + constexpr int kReturn3 = 3; + constexpr int kReturn5 = 5; + + frame.EnsureReturnSlots(kReturn3); + + // Allocate a double slot, which must add 1 padding slot. This should occupy + // slots 4 and 5, and AllocateSpillSlot returns the last slot index. + EXPECT_EQ(kFixed3 + 2, frame.AllocateSpillSlot(2 * kSlotSize, 2 * kSlotSize)); + EXPECT_EQ(kFixed3 + kReturn3 + 3, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(3, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn3, frame.GetReturnSlotCount()); + + frame.EnsureReturnSlots(kReturn5); + + // Allocate a single slot, which should take the padding slot. + EXPECT_EQ(kFixed3, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + EXPECT_EQ(kFixed3 + kReturn5 + 3, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(3, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn5, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSpillSlotAndEndSpillArea) { + Frame frame(kFixed3); + + // Allocate a double slot, which must add 1 padding slot. + EXPECT_EQ(kFixed3 + 2, frame.AllocateSpillSlot(2 * kSlotSize, 2 * kSlotSize)); + + // Allocate an unaligned double slot. This should be at the end. + EXPECT_EQ(kFixed3 + 4, frame.AllocateSpillSlot(2 * kSlotSize)); + EXPECT_EQ(kFixed3 + 5, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(5, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + // Allocate a single slot. This should not be the padding slot, since that + // area has been closed by the unaligned allocation. + EXPECT_EQ(kFixed3 + 5, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + EXPECT_EQ(kFixed3 + 6, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(6, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSpillSlotOverAligned) { + Frame frame(kFixed1); + + // Allocate a 4-aligned double slot, which must add 3 padding slots. This + // also terminates the slot area. Returns the starting slot in this case. + EXPECT_EQ(kFixed1 + 4, frame.AllocateSpillSlot(2 * kSlotSize, 4 * kSlotSize)); + EXPECT_EQ(kFixed1 + 5, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(5, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); + + // Allocate a single slot. This should not use any padding slot. + EXPECT_EQ(kFixed1 + 5, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + EXPECT_EQ(kFixed1 + 6, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(6, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AllocateSpillSlotUnderAligned) { + Frame frame(kFixed1); + + // Allocate a 1-aligned double slot. This also terminates the slot area. + EXPECT_EQ(kFixed1 + 1, frame.AllocateSpillSlot(2 * kSlotSize, kSlotSize)); + EXPECT_EQ(kFixed1 + 2, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed1, frame.GetFixedSlotCount()); + EXPECT_EQ(2, frame.GetSpillSlotCount()); + EXPECT_EQ(0, frame.GetReturnSlotCount()); +} + +TEST_F(FrameTest, AlignFrame) { + Frame frame(kFixed3); + constexpr int kReturn3 = 3; + + frame.EnsureReturnSlots(kReturn3); + + // Allocate two single slots, which leaves spill slots not 2-aligned. + EXPECT_EQ(kFixed3, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + EXPECT_EQ(kFixed3 + 1, frame.AllocateSpillSlot(kSlotSize, kSlotSize)); + + // Align to 2 slots. This should pad the spill and return slot areas. + frame.AlignFrame(2 * kSlotSize); + + EXPECT_EQ(kFixed3 + 3 + kReturn3 + 1, frame.GetTotalFrameSlotCount()); + EXPECT_EQ(kFixed3, frame.GetFixedSlotCount()); + EXPECT_EQ(3, frame.GetSpillSlotCount()); + EXPECT_EQ(kReturn3 + 1, frame.GetReturnSlotCount()); +} + +} // namespace compiler +} // namespace internal +} // namespace v8 diff --git a/test/unittests/wasm/wasm-compiler-unittest.cc b/test/unittests/wasm/wasm-compiler-unittest.cc index 9689a15eb4..69e4c30053 100644 --- a/test/unittests/wasm/wasm-compiler-unittest.cc +++ b/test/unittests/wasm/wasm-compiler-unittest.cc @@ -2,13 +2,14 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "test/unittests/test-utils.h" +#include "src/compiler/wasm-compiler.h" #include "src/codegen/machine-type.h" #include "src/codegen/signature.h" #include "src/compiler/linkage.h" -#include "src/compiler/wasm-compiler.h" #include "src/wasm/value-type.h" +#include "src/wasm/wasm-linkage.h" +#include "test/unittests/test-utils.h" namespace v8 { namespace internal { @@ -66,6 +67,45 @@ TEST_F(WasmCallDescriptorTest, TestExternRefIsGrouped) { } } +TEST_F(WasmCallDescriptorTest, Regress_1174500) { + // Our test signature should have just enough params and returns to force + // 1 param and 1 return to be allocated as stack slots. Use FP registers to + // avoid interference with implicit parameters, like the Wasm Instance. + constexpr int kParamRegisters = arraysize(kFpParamRegisters); + constexpr int kParams = kParamRegisters + 1; + constexpr int kReturnRegisters = arraysize(kFpReturnRegisters); + constexpr int kReturns = kReturnRegisters + 1; + ValueType types[kReturns + kParams]; + // One S128 return slot which shouldn't be padded unless the arguments area + // of the frame requires it. + for (int i = 0; i < kReturnRegisters; ++i) types[i] = kWasmF32; + types[kReturnRegisters] = kWasmS128; + // One F32 parameter slot to misalign the parameter area. + for (int i = 0; i < kParamRegisters; ++i) types[kReturns + i] = kWasmF32; + types[kReturns + kParamRegisters] = kWasmF32; + + FunctionSig sig(kReturns, kParams, types); + compiler::CallDescriptor* desc = + compiler::GetWasmCallDescriptor(zone(), &sig); + + // Get the location of our stack parameter slot. Skip the implicit Wasm + // instance parameter. + compiler::LinkageLocation last_param = desc->GetInputLocation(kParams + 1); + EXPECT_TRUE(last_param.IsCallerFrameSlot()); + EXPECT_EQ(MachineType::Float32(), last_param.GetType()); + EXPECT_EQ(-1, last_param.GetLocation()); + + // The stack return slot should be right above our last parameter, and any + // argument padding slots. The return slot itself should not be padded. + const int padding = kPadArguments ? 1 : 0; + const int first_return_slot = -1 - (padding + 1); + compiler::LinkageLocation return_location = + desc->GetReturnLocation(kReturns - 1); + EXPECT_TRUE(return_location.IsCallerFrameSlot()); + EXPECT_EQ(MachineType::Simd128(), return_location.GetType()); + EXPECT_EQ(first_return_slot, return_location.GetLocation()); +} + } // namespace wasm } // namespace internal } // namespace v8