Skip to content

V
V8
Commit 9a540436 authored by Bill Budge's avatar Bill Budge Committed by Commit Bot
Browse files
Options

Revert "[compiler][wasm] Fix Wasm linkage" 

This reverts commit 0818d138.

Reason for revert: Rolling back to previous greedy slot allocator.

tbr=ahaas@chromium.org

Original change's description:
> [compiler][wasm] Fix Wasm linkage
>
> - Fixes a problem when constructing Wasm CallDescriptors, where the
>   allocation tries to treat parameters and returns as if they are in the
>   same frame. This doesn't work when slots may be aligned in their
>   frame. Instead, allocate parameters and returns separately and offset
>   return slots by the number of parameter slots.
> - Adds argument slot padding in the CallDescriptor lowering case, to
>   prepare for when 32 bit targets align stack frames and require
>   padding.
> - Adds a regression test.
>
> Bug: chromium:1174500
> Change-Id: I60d96a94b171a0d27ff61cbab35623976b0c6da8
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2683024
> Commit-Queue: Bill Budge <bbudge@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#72588}

Bug: chromium:1174500
Change-Id: I1d1c389acde43bd56e6d2a27e1a3eb8ea4d6073c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2713206
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: 's avatarBill Budge <bbudge@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72934}
parent 93bcd62e
Show whitespace changes
Inline Side-by-side
Showing with 35 additions and 84 deletions
src/compiler/wasm-compiler.cc
View file @ 9a540436
......@@ -8144,9 +8144,8 @@ class LinkageLocationAllocator {
public:
template <size_t kNumGpRegs, size_t kNumFpRegs>
constexpr LinkageLocationAllocator(const Register (&gp)[kNumGpRegs],
const DoubleRegister (&fp)[kNumFpRegs],
int slot_offset)
: allocator_(wasm::LinkageAllocator(gp, fp)), slot_offset_(slot_offset) {}
const DoubleRegister (&fp)[kNumFpRegs])
: allocator_(wasm::LinkageAllocator(gp, fp)) {}
LinkageLocation Next(MachineRepresentation rep) {
MachineType type = MachineType::TypeForRepresentation(rep);
......@@ -8160,19 +8159,16 @@ class LinkageLocationAllocator {
return LinkageLocation::ForRegister(reg_code, type);
}
// Cannot use register; use stack slot.
int index = -1 - (slot_offset_ + allocator_.NextStackSlot(rep));
int index = -1 - 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
......@@ -8190,8 +8186,8 @@ CallDescriptor* GetWasmCallDescriptor(
fsig->parameter_count() + extra_params);
// Add register and/or stack parameter(s).
LinkageLocationAllocator params(
wasm::kGpParamRegisters, wasm::kFpParamRegisters, 0 /* no slot offset */);
LinkageLocationAllocator params(wasm::kGpParamRegisters,
wasm::kFpParamRegisters);
// The instance object.
locations.AddParam(params.Next(MachineRepresentation::kTaggedPointer));
......@@ -8227,21 +8223,22 @@ 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++;
// Add return location(s).
LinkageLocationAllocator rets(wasm::kGpReturnRegisters,
wasm::kFpReturnRegisters, parameter_slots);
rets.SetStackOffset(parameter_slots);
const int return_count = static_cast<int>(locations.return_count_);
for (int i = 0; i < return_count; i++) {
MachineRepresentation ret = fsig->GetReturn(i).machine_representation();
locations.AddReturn(rets.Next(ret));
auto l = rets.Next(ret);
locations.AddReturn(l);
}
int return_slots = rets.NumStackSlots();
const RegList kCalleeSaveRegisters = 0;
const RegList kCalleeSaveFPRegisters = 0;
......@@ -8268,7 +8265,7 @@ CallDescriptor* GetWasmCallDescriptor(
target_type, // target MachineType
target_loc, // target location
locations.Build(), // location_sig
parameter_slots, // parameter slot count
parameter_slots, // stack_parameter_count
compiler::Operator::kNoProperties, // properties
kCalleeSaveRegisters, // callee-saved registers
kCalleeSaveFPRegisters, // callee-saved fp regs
......@@ -8276,7 +8273,7 @@ CallDescriptor* GetWasmCallDescriptor(
"wasm-call", // debug name
StackArgumentOrder::kDefault, // order of the arguments in the stack
0, // allocatable registers
return_slots); // return slot count
rets.NumStackSlots() - parameter_slots); // stack_return_count
}
namespace {
......@@ -8309,9 +8306,8 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
(call_descriptor->GetInputLocation(call_descriptor->InputCount() - 1) ==
LinkageLocation::ForRegister(kJSFunctionRegister.code(),
MachineType::TaggedPointer()));
LinkageLocationAllocator params(
wasm::kGpParamRegisters, wasm::kFpParamRegisters, 0 /* no slot offset */);
LinkageLocationAllocator params(wasm::kGpParamRegisters,
wasm::kFpParamRegisters);
for (size_t i = 0, e = call_descriptor->ParameterCount() -
(has_callable_param ? 1 : 0);
i < e; i++) {
......@@ -8329,12 +8325,9 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
kJSFunctionRegister.code(), MachineType::TaggedPointer()));
}
int parameter_slots = params.NumStackSlots();
if (ShouldPadArguments(parameter_slots)) parameter_slots++;
LinkageLocationAllocator rets(wasm::kGpReturnRegisters,
wasm::kFpReturnRegisters, parameter_slots);
wasm::kFpReturnRegisters);
rets.SetStackOffset(params.NumStackSlots());
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++) {
......@@ -8346,14 +8339,12 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
}
}
int return_slots = rets.NumStackSlots();
return zone->New<CallDescriptor>( // --
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
params.NumStackSlots(), // stack_parameter_count
call_descriptor->properties(), // properties
call_descriptor->CalleeSavedRegisters(), // callee-saved registers
call_descriptor->CalleeSavedFPRegisters(), // callee-saved fp regs
......@@ -8361,7 +8352,7 @@ CallDescriptor* ReplaceTypeInCallDescriptorWith(
call_descriptor->debug_name(), // debug name
call_descriptor->GetStackArgumentOrder(), // stack order
call_descriptor->AllocatableRegisters(), // allocatable registers
return_slots); // return slot count
rets.NumStackSlots() - params.NumStackSlots()); // stack_return_count
}
} // namespace
......
test/unittests/wasm/wasm-compiler-unittest.cc
View file @ 9a540436
......@@ -2,14 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/compiler/wasm-compiler.h"
#include "test/unittests/test-utils.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 {
......@@ -67,45 +66,6 @@ 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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment