[liftoff] Fix temp register for BrImpl with TierupCheck

Allocating a temp register in a conditional branch confuses
the LiftoffAssembler's state tracking, so this patch moves
allocation of the register into the unconditional part of the
control flow.

Fixed: chromium:1275711
Change-Id: Ic83ba8c098c5edb33d035c1a93931d54cc1f1caa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3320423
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78275}
This commit is contained in:
Jakob Kummerow 2021-12-07 14:26:35 +01:00 committed by V8 LUCI CQ
parent 94f86e6d4e
commit 649c980588

View File

@ -721,14 +721,16 @@ class LiftoffCompiler {
}
void TierupCheck(FullDecoder* decoder, WasmCodePosition position,
int budget_used) {
int budget_used, Register scratch_reg) {
if (for_debugging_ != kNoDebugging) return;
CODE_COMMENT("tierup check");
// We never want to blow the entire budget at once.
const int kMax = FLAG_wasm_tiering_budget / 4;
if (budget_used > kMax) budget_used = kMax;
LiftoffRegister budget_reg = __ GetUnusedRegister(kGpReg, {});
LiftoffRegister budget_reg = scratch_reg == no_reg
? __ GetUnusedRegister(kGpReg, {})
: LiftoffRegister(scratch_reg);
__ Fill(budget_reg, liftoff::kTierupBudgetOffset, ValueKind::kI32);
LiftoffRegList regs_to_save = __ cache_state()->used_registers;
// The cached instance will be reloaded separately.
@ -2235,7 +2237,7 @@ class LiftoffCompiler {
void TierupCheckOnExit(FullDecoder* decoder) {
if (!dynamic_tiering()) return;
TierupCheck(decoder, decoder->position(), __ pc_offset());
TierupCheck(decoder, decoder->position(), __ pc_offset(), no_reg);
LiftoffRegList pinned;
LiftoffRegister budget = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
LiftoffRegister array = pinned.set(__ GetUnusedRegister(kGpReg, pinned));
@ -2580,17 +2582,12 @@ class LiftoffCompiler {
__ PushRegister(kind, dst);
}
void BrImpl(FullDecoder* decoder, Control* target) {
if (!target->br_merge()->reached) {
target->label_state.InitMerge(
*__ cache_state(), __ num_locals(), target->br_merge()->arity,
target->stack_depth + target->num_exceptions);
}
void BrImpl(FullDecoder* decoder, Control* target, Register scratch_reg) {
if (dynamic_tiering()) {
if (target->is_loop()) {
DCHECK(target->label.get()->is_bound());
int jump_distance = __ pc_offset() - target->label.get()->pos();
TierupCheck(decoder, decoder->position(), jump_distance);
TierupCheck(decoder, decoder->position(), jump_distance, scratch_reg);
} else {
// To estimate time spent in this function more accurately, we could
// increment the tiering budget on forward jumps. However, we don't
@ -2598,6 +2595,11 @@ class LiftoffCompiler {
// and found to not make a difference.
}
}
if (!target->br_merge()->reached) {
target->label_state.InitMerge(
*__ cache_state(), __ num_locals(), target->br_merge()->arity,
target->stack_depth + target->num_exceptions);
}
__ MergeStackWith(target->label_state, target->br_merge()->arity,
target->is_loop() ? LiftoffAssembler::kBackwardJump
: LiftoffAssembler::kForwardJump);
@ -2606,10 +2608,14 @@ class LiftoffCompiler {
void BrOrRet(FullDecoder* decoder, uint32_t depth,
uint32_t /* drop_values */) {
BrOrRetImpl(decoder, depth, no_reg);
}
void BrOrRetImpl(FullDecoder* decoder, uint32_t depth, Register scratch_reg) {
if (depth == decoder->control_depth() - 1) {
DoReturn(decoder, 0);
} else {
BrImpl(decoder, decoder->control_at(depth));
BrImpl(decoder, decoder->control_at(depth), scratch_reg);
}
}
@ -2622,12 +2628,16 @@ class LiftoffCompiler {
decoder->control_at(depth)->br_merge()->arity);
}
Register scratch_reg = no_reg;
if (dynamic_tiering()) {
scratch_reg = __ GetUnusedRegister(kGpReg, {}).gp();
}
Label cont_false;
// Test the condition on the value stack, jump to {cont_false} if zero.
JumpIfFalse(decoder, &cont_false);
BrOrRet(decoder, depth, 0);
BrOrRetImpl(decoder, depth, scratch_reg);
__ bind(&cont_false);
}