Commit acc4ed88 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[Liftoff] Avoid unneeded CacheState Steal

When finishing a one-armed if, the else state has to be merged into the
end state. We did this before by switching to the else state, then
doing the merge. This CL changes this to avoid the switch.

Drive-by: Add a few missing "const" qualifiers. The style guide forbids
non-const l-value references.

R=titzer@chromium.org

Bug: v8:8423, v8:6600
Change-Id: Iab2aeca393147fba55493bebabd27bc4d77baa0f
Reviewed-on: https://chromium-review.googlesource.com/c/1375656Reviewed-by: 's avatarBen Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58222}
parent 9c0a4858
......@@ -153,9 +153,11 @@ class StackTransferRecipe {
}
void TransferStackSlot(const LiftoffAssembler::CacheState& dst_state,
uint32_t dst_index, uint32_t src_index) {
uint32_t dst_index,
const LiftoffAssembler::CacheState& src_state,
uint32_t src_index) {
const VarState& dst = dst_state.stack_state[dst_index];
const VarState& src = __ cache_state()->stack_state[src_index];
const VarState& src = src_state.stack_state[src_index];
DCHECK_EQ(dst.type(), src.type());
switch (dst.loc()) {
case VarState::kStack:
......@@ -329,7 +331,7 @@ void LiftoffAssembler::CacheState::InitMerge(const CacheState& source,
last_spilled_regs = source.last_spilled_regs;
}
void LiftoffAssembler::CacheState::Steal(CacheState& source) {
void LiftoffAssembler::CacheState::Steal(const CacheState& source) {
// Just use the move assignment operator.
*this = std::move(source);
}
......@@ -386,17 +388,19 @@ LiftoffRegister LiftoffAssembler::PopToRegister(LiftoffRegList pinned) {
UNREACHABLE();
}
void LiftoffAssembler::MergeFullStackWith(CacheState& target) {
DCHECK_EQ(cache_state_.stack_height(), target.stack_height());
void LiftoffAssembler::MergeFullStackWith(const CacheState& target,
const CacheState& source) {
DCHECK_EQ(source.stack_height(), target.stack_height());
// TODO(clemensh): Reuse the same StackTransferRecipe object to save some
// allocations.
StackTransferRecipe transfers(this);
for (uint32_t i = 0, e = cache_state_.stack_height(); i < e; ++i) {
transfers.TransferStackSlot(target, i, i);
for (uint32_t i = 0, e = source.stack_height(); i < e; ++i) {
transfers.TransferStackSlot(target, i, source, i);
}
}
void LiftoffAssembler::MergeStackWith(CacheState& target, uint32_t arity) {
void LiftoffAssembler::MergeStackWith(const CacheState& target,
uint32_t arity) {
// Before: ----------------|------ pop_count -----|--- arity ---|
// ^target_stack_height ^stack_base ^stack_height
// After: ----|-- arity --|
......@@ -410,10 +414,11 @@ void LiftoffAssembler::MergeStackWith(CacheState& target, uint32_t arity) {
uint32_t target_stack_base = target_stack_height - arity;
StackTransferRecipe transfers(this);
for (uint32_t i = 0; i < target_stack_base; ++i) {
transfers.TransferStackSlot(target, i, i);
transfers.TransferStackSlot(target, i, cache_state_, i);
}
for (uint32_t i = 0; i < arity; ++i) {
transfers.TransferStackSlot(target, target_stack_base + i, stack_base + i);
transfers.TransferStackSlot(target, target_stack_base + i, cache_state_,
stack_base + i);
}
}
......
......@@ -232,7 +232,7 @@ class LiftoffAssembler : public TurboAssembler {
void InitMerge(const CacheState& source, uint32_t num_locals,
uint32_t arity, uint32_t stack_depth);
void Steal(CacheState& source);
void Steal(const CacheState& source);
void Split(const CacheState& source);
......@@ -298,8 +298,8 @@ class LiftoffAssembler : public TurboAssembler {
return SpillOneRegister(candidates, pinned);
}
void MergeFullStackWith(CacheState&);
void MergeStackWith(CacheState&, uint32_t arity);
void MergeFullStackWith(const CacheState& target, const CacheState& source);
void MergeStackWith(const CacheState& target, uint32_t arity);
void Spill(uint32_t index);
void SpillLocals();
......
......@@ -490,14 +490,14 @@ class LiftoffCompiler {
void FallThruTo(FullDecoder* decoder, Control* c) {
if (c->end_merge.reached) {
__ MergeFullStackWith(c->label_state);
__ MergeFullStackWith(c->label_state, *__ cache_state());
} else if (c->is_onearmed_if()) {
// Init the merge point from the else state, then merge the if state into
// that.
DCHECK_EQ(0, c->end_merge.arity);
c->label_state.InitMerge(c->else_state->state, __ num_locals(), 0,
c->stack_depth);
__ MergeFullStackWith(c->label_state);
__ MergeFullStackWith(c->label_state, *__ cache_state());
} else {
c->label_state.Split(*__ cache_state());
}
......@@ -505,14 +505,14 @@ class LiftoffCompiler {
}
void PopControl(FullDecoder* decoder, Control* c) {
// A loop just falls through.
if (c->is_loop()) return;
if (c->is_onearmed_if()) {
if (c->end_merge.reached) {
// Generate the code to merge the else state into the end state.
// TODO(clemensh): Do this without switching to the else state first.
__ emit_jump(c->label.get());
__ bind(c->else_state->label.get());
__ cache_state()->Steal(c->else_state->state);
__ MergeFullStackWith(c->label_state);
__ MergeFullStackWith(c->label_state, c->else_state->state);
__ cache_state()->Steal(c->label_state);
} else {
// There is no merge at the end of the if, so just continue with the
......@@ -520,12 +520,10 @@ class LiftoffCompiler {
__ bind(c->else_state->label.get());
__ cache_state()->Steal(c->else_state->state);
}
} else if (!c->is_loop() && c->end_merge.reached) {
} else if (c->end_merge.reached) {
__ cache_state()->Steal(c->label_state);
}
if (!c->label.get()->is_bound()) {
__ bind(c->label.get());
}
if (!c->label.get()->is_bound()) __ bind(c->label.get());
}
void EndControl(FullDecoder* decoder, Control* c) {}
......@@ -1334,7 +1332,7 @@ class LiftoffCompiler {
c->label_state.InitMerge(*__ cache_state(), __ num_locals(),
c->end_merge.arity, c->stack_depth);
}
__ MergeFullStackWith(c->label_state);
__ MergeFullStackWith(c->label_state, *__ cache_state());
__ emit_jump(c->label.get());
}
__ bind(c->else_state->label.get());
......
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