Revert "[Liftoff] Fix register use count"
This reverts commit ada648006b
.
Reason for revert: Failure with slow dchecks: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20debug/20982
Original change's description:
> [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/1111958
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#53976}
TBR=titzer@chromium.org,clemensh@chromium.org
Change-Id: I5b8d8d405dcd7f82ee431cba290419425b9859a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:854050, v8:6600
Reviewed-on: https://chromium-review.googlesource.com/1112277
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53979}
This commit is contained in:
parent
aafd5c52ab
commit
cf2f6a57b5
@ -4,8 +4,6 @@
|
||||
|
||||
#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"
|
||||
@ -594,34 +592,6 @@ 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,9 +330,6 @@ 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,7 +430,6 @@ class LiftoffCompiler {
|
||||
|
||||
void NextInstruction(Decoder* decoder, WasmOpcode opcode) {
|
||||
TraceCacheState(decoder);
|
||||
SLOW_DCHECK(__ ValidateCacheState());
|
||||
DEBUG_CODE_COMMENT(WasmOpcodes::OpcodeName(opcode));
|
||||
}
|
||||
|
||||
@ -1077,7 +1076,6 @@ class LiftoffCompiler {
|
||||
return;
|
||||
}
|
||||
state.dec_used(slot_reg);
|
||||
dst_slot.MakeStack();
|
||||
}
|
||||
DCHECK_EQ(type, __ local_type(local_index));
|
||||
RegClass rc = reg_class_for(type);
|
||||
|
@ -1,28 +0,0 @@
|
||||
// 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();
|
Loading…
Reference in New Issue
Block a user