Commit 456e5687 authored by Joey Gouly's avatar Joey Gouly Committed by Commit Bot

[arm64] Avoid padding poke when unnecessary

This also fixes a bug in 'InitializeCallBuffer', where it wouldn't claim enough
slots for each parameter. This caused the Simd128 instruction selector test to
only claim 3 slots (rather than 4) and then perform an unnecessary padding poke.

v8_Default_embedded_blob_size from the generated file gen/embedded.S
  Before: 4957056
   After: 4954368

This gives a 0.05% size decrease.

Change-Id: Ic9bb998fb8a9111fb90e1c3e537ea0f2a5fa7b33
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1617665Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Martyn Capewell <martyn.capewell@arm.com>
Cr-Commit-Position: refs/heads/master@{#61649}
parent 61523c45
...@@ -1701,6 +1701,7 @@ void InstructionSelector::EmitPrepareArguments( ...@@ -1701,6 +1701,7 @@ void InstructionSelector::EmitPrepareArguments(
// `arguments` includes alignment "holes". This means that slots bigger than // `arguments` includes alignment "holes". This means that slots bigger than
// kSystemPointerSize, e.g. Simd128, will span across multiple arguments. // kSystemPointerSize, e.g. Simd128, will span across multiple arguments.
int claim_count = static_cast<int>(arguments->size()); int claim_count = static_cast<int>(arguments->size());
bool needs_padding = claim_count % 2 != 0;
int slot = claim_count - 1; int slot = claim_count - 1;
claim_count = RoundUp(claim_count, 2); claim_count = RoundUp(claim_count, 2);
// Bump the stack pointer. // Bump the stack pointer.
...@@ -1710,9 +1711,10 @@ void InstructionSelector::EmitPrepareArguments( ...@@ -1710,9 +1711,10 @@ void InstructionSelector::EmitPrepareArguments(
// and emit paired stores with increment for non c frames. // and emit paired stores with increment for non c frames.
Emit(kArm64Claim, g.NoOutput(), g.TempImmediate(claim_count)); Emit(kArm64Claim, g.NoOutput(), g.TempImmediate(claim_count));
// Store padding, which might be overwritten. if (needs_padding) {
Emit(kArm64Poke, g.NoOutput(), g.UseImmediate(0), Emit(kArm64Poke, g.NoOutput(), g.UseImmediate(0),
g.TempImmediate(claim_count - 1)); g.TempImmediate(claim_count - 1));
}
} }
// Poke the arguments into the stack. // Poke the arguments into the stack.
......
...@@ -1006,8 +1006,13 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer, ...@@ -1006,8 +1006,13 @@ void InstructionSelector::InitializeCallBuffer(Node* call, CallBuffer* buffer,
UnallocatedOperand unallocated = UnallocatedOperand::cast(op); UnallocatedOperand unallocated = UnallocatedOperand::cast(op);
if (unallocated.HasFixedSlotPolicy() && !call_tail) { if (unallocated.HasFixedSlotPolicy() && !call_tail) {
int stack_index = -unallocated.fixed_slot_index() - 1; int stack_index = -unallocated.fixed_slot_index() - 1;
// This can insert empty slots before stack_index and will insert enough
// slots after stack_index to store the parameter.
if (static_cast<size_t>(stack_index) >= buffer->pushed_nodes.size()) { if (static_cast<size_t>(stack_index) >= buffer->pushed_nodes.size()) {
buffer->pushed_nodes.resize(stack_index + 1); int num_slots = std::max(
1, (ElementSizeInBytes(location.GetType().representation()) /
kSystemPointerSize));
buffer->pushed_nodes.resize(stack_index + num_slots);
} }
PushParameter param = {*iter, location}; PushParameter param = {*iter, location};
buffer->pushed_nodes[stack_index] = param; buffer->pushed_nodes[stack_index] = param;
......
...@@ -4648,9 +4648,7 @@ void TestPokePair(InstructionSelectorTest::StreamBuilder& m, Zone* zone, ...@@ -4648,9 +4648,7 @@ void TestPokePair(InstructionSelectorTest::StreamBuilder& m, Zone* zone,
} }
EXPECT_EQ(expected_poke_pair, num_poke_pair); EXPECT_EQ(expected_poke_pair, num_poke_pair);
// Note: The `+ 1` here comes from the padding Poke in EXPECT_EQ(expected_poke, num_poke);
// EmitPrepareArguments.
EXPECT_EQ(expected_poke + 1, num_poke);
} }
} // namespace } // namespace
...@@ -4670,7 +4668,9 @@ TEST_F(InstructionSelectorTest, PokePairPrepareArgumentsInt32) { ...@@ -4670,7 +4668,9 @@ TEST_F(InstructionSelectorTest, PokePairPrepareArgumentsInt32) {
}; };
const int expected_poke_pair = 1; const int expected_poke_pair = 1;
const int expected_poke = 1; // Note: The `+ 1` here comes from the padding Poke in
// EmitPrepareArguments.
const int expected_poke = 1 + 1;
TestPokePair(m, zone(), builder, nodes, arraysize(nodes), TestPokePair(m, zone(), builder, nodes, arraysize(nodes),
expected_poke_pair, expected_poke); expected_poke_pair, expected_poke);
...@@ -4795,7 +4795,10 @@ TEST_F(InstructionSelectorTest, PokePairPrepareArgumentsIntFloatMixed) { ...@@ -4795,7 +4795,10 @@ TEST_F(InstructionSelectorTest, PokePairPrepareArgumentsIntFloatMixed) {
m.Float64Constant(0.0f), m.Float64Constant(0.0f)}; m.Float64Constant(0.0f), m.Float64Constant(0.0f)};
const int expected_poke_pair = 2; const int expected_poke_pair = 2;
const int expected_poke = 3;
// Note: The `+ 1` here comes from the padding Poke in
// EmitPrepareArguments.
const int expected_poke = 3 + 1;
TestPokePair(m, zone(), builder, nodes, arraysize(nodes), TestPokePair(m, zone(), builder, nodes, arraysize(nodes),
expected_poke_pair, expected_poke); expected_poke_pair, expected_poke);
......
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