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

[Liftoff] Fix register use count

In {SetLocalFromStackSlot}, we decrement the use count of the register
in the target slot without updating this slot, and then call
{GetUnusedRegister}. At that point, the register use counts do not
match the cache state, which leads to errors later on.
This CL fixes this by marking the target slot as a stack slot after
reducing the register use count.

It also adds a Validation which helped to find that error and will
catch similar errors earlier.

R=titzer@chromium.org

Bug: chromium:854050, v8:6600
Change-Id: I74d3a5aa947ec4247d7b4557567f642bf4082316
Reviewed-on: https://chromium-review.googlesource.com/1111958Reviewed-by: 's avatarBen Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53976}
parent c31cf146
......@@ -4,6 +4,8 @@
#include "src/wasm/baseline/liftoff-assembler.h"
#include <sstream>
#include "src/assembler-inl.h"
#include "src/compiler/linkage.h"
#include "src/compiler/wasm-compiler.h"
......@@ -592,6 +594,34 @@ void LiftoffAssembler::ParallelRegisterMove(
}
}
bool LiftoffAssembler::ValidateCacheState() const {
uint32_t register_use_count[kAfterMaxLiftoffRegCode] = {0};
LiftoffRegList used_regs;
for (const VarState& var : cache_state_.stack_state) {
if (!var.is_reg()) continue;
++register_use_count[var.reg().liftoff_code()];
used_regs.set(var.reg());
}
bool valid = memcmp(register_use_count, cache_state_.register_use_count,
sizeof(register_use_count)) == 0 &&
used_regs == cache_state_.used_registers;
if (valid) return true;
std::ostringstream os;
os << "Error in LiftoffAssembler::ValidateCacheState().\n";
os << "expected: used_regs " << used_regs << ", counts [";
for (int reg = 0; reg < kAfterMaxLiftoffRegCode; ++reg) {
os << (reg == 0 ? "" : ", ") << register_use_count[reg];
}
os << "]\n";
os << "found: used_regs " << cache_state_.used_registers << ", counts [";
for (int reg = 0; reg < kAfterMaxLiftoffRegCode; ++reg) {
os << (reg == 0 ? "" : ", ") << cache_state_.register_use_count[reg];
}
os << "]\n";
os << "Use --trace-liftoff to debug.\n";
FATAL("%s", os.str().c_str());
}
LiftoffRegister LiftoffAssembler::SpillOneRegister(LiftoffRegList candidates,
LiftoffRegList pinned) {
// Spill one cached value to free a register.
......
......@@ -330,6 +330,9 @@ class LiftoffAssembler : public TurboAssembler {
};
void ParallelRegisterMove(std::initializer_list<ParallelRegisterMoveTuple>);
// Validate that the register use counts reflect the state of the cache.
bool ValidateCacheState() const;
////////////////////////////////////
// Platform-specific part. //
////////////////////////////////////
......
......@@ -430,6 +430,7 @@ class LiftoffCompiler {
void NextInstruction(Decoder* decoder, WasmOpcode opcode) {
TraceCacheState(decoder);
SLOW_DCHECK(__ ValidateCacheState());
DEBUG_CODE_COMMENT(WasmOpcodes::OpcodeName(opcode));
}
......@@ -1076,6 +1077,7 @@ class LiftoffCompiler {
return;
}
state.dec_used(slot_reg);
dst_slot.MakeStack();
}
DCHECK_EQ(type, __ local_type(local_index));
RegClass rc = reg_class_for(type);
......
// Copyright 2018 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.
load('test/mjsunit/wasm/wasm-constants.js');
load('test/mjsunit/wasm/wasm-module-builder.js');
const builder = new WasmModuleBuilder();
builder.addFunction(undefined, makeSig([kWasmI32, kWasmF32], []))
.addLocals({i32_count: 7})
.addBody([
kExprGetLocal, 0, // get_local
kExprI32Const, 0, // i32.const 0
kExprIf, kWasmStmt, // if
kExprUnreachable, // unreachable
kExprEnd, // end if
kExprGetLocal, 4, // get_local
kExprTeeLocal, 8, // tee_local
kExprBrIf, 0, // br_if depth=0
kExprTeeLocal, 7, // tee_local
kExprTeeLocal, 0, // tee_local
kExprTeeLocal, 2, // tee_local
kExprTeeLocal, 8, // tee_local
kExprDrop, // drop
kExprLoop, kWasmStmt, // loop
kExprEnd, // end loop
]);
builder.instantiate();
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