Commit 42b4f15a authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[liftoff] Fix missing stack move

The {operator==} on {VarState} did not check the spill offset, so when
merging stack states, we forgot to move stack values if both source and
destination were stack slots, but at different offsets.
This CL fixes this by removing the {operator==}, because the semantics
(and use) are not clear, and it's only used in one place anyway.
The equality check was mostly redundant, so inlining it also makes the
code smaller and faster.

R=ahaas@chromium.org

Bug: v8:10702
Change-Id: I6c8b2cfd1002274175c9a17d305692e4631fd7dc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2304574Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68916}
parent 5c84b6be
...@@ -84,16 +84,20 @@ class StackTransferRecipe { ...@@ -84,16 +84,20 @@ class StackTransferRecipe {
V8_INLINE void TransferStackSlot(const VarState& dst, const VarState& src) { V8_INLINE void TransferStackSlot(const VarState& dst, const VarState& src) {
DCHECK_EQ(dst.type(), src.type()); DCHECK_EQ(dst.type(), src.type());
if (dst == src) return;
if (dst.is_reg()) { if (dst.is_reg()) {
LoadIntoRegister(dst.reg(), src, src.offset()); LoadIntoRegister(dst.reg(), src, src.offset());
return; return;
} }
if (dst.is_const()) {
DCHECK_EQ(dst.i32_const(), src.i32_const());
return;
}
DCHECK(dst.is_stack()); DCHECK(dst.is_stack());
switch (src.loc()) { switch (src.loc()) {
case VarState::kStack: case VarState::kStack:
DCHECK_NE(src.offset(), dst.offset()); if (src.offset() != dst.offset()) {
asm_->MoveStackValue(dst.offset(), src.offset(), src.type()); asm_->MoveStackValue(dst.offset(), src.offset(), src.type());
}
break; break;
case VarState::kRegister: case VarState::kRegister:
asm_->Spill(dst.offset(), src.reg(), src.type()); asm_->Spill(dst.offset(), src.reg(), src.type());
......
...@@ -56,20 +56,6 @@ class LiftoffAssembler : public TurboAssembler { ...@@ -56,20 +56,6 @@ class LiftoffAssembler : public TurboAssembler {
DCHECK(type_ == kWasmI32 || type_ == kWasmI64); DCHECK(type_ == kWasmI32 || type_ == kWasmI64);
} }
bool operator==(const VarState& other) const {
if (loc_ != other.loc_) return false;
if (type_ != other.type_) return false;
switch (loc_) {
case kStack:
return true;
case kRegister:
return reg_ == other.reg_;
case kIntConst:
return i32_const_ == other.i32_const_;
}
UNREACHABLE();
}
bool is_stack() const { return loc_ == kStack; } bool is_stack() const { return loc_ == kStack; }
bool is_gp_reg() const { return loc_ == kRegister && reg_.is_gp(); } bool is_gp_reg() const { return loc_ == kRegister && reg_.is_gp(); }
bool is_fp_reg() const { return loc_ == kRegister && reg_.is_fp(); } bool is_fp_reg() const { return loc_ == kRegister && reg_.is_fp(); }
......
// Copyright 2020 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-module-builder.js');
const builder = new WasmModuleBuilder();
builder.addGlobal(kWasmI32, 1).init = 35;
builder.addType(makeSig([], [kWasmI32]));
builder.addType(makeSig([kWasmI32, kWasmI32], [kWasmI32]));
// Generate function 1 (out of 3).
builder.addFunction(undefined, 0 /* sig */).addBody([
// signature: i_v
// body:
kExprI32Const, 1, // i32.const
]);
// Generate function 2 (out of 3).
builder.addFunction(undefined, 0 /* sig */).addBody([
// signature: i_v
// body:
kExprI32Const, 0, // i32.const
]);
// Generate function 3 (out of 3).
builder.addFunction(undefined, 1 /* sig */).addBody([
// signature: i_ii
// body:
kExprBlock, kWasmI32, // block @1 i32
kExprF64Const, 0, 0, 0, 0, 0, 0, 0, 0, // f64.const
kExprI32SConvertF64, // i32.trunc_f64_s
kExprCallFunction, 1, // call function #1: i_v
kExprBrIf, 0, // br_if depth=0
kExprGlobalGet, 0, // global.get
kExprCallFunction, 0, // call function #0: i_v
kExprBrIf, 0, // br_if depth=0
kExprI32ShrS, // i32.shr_s
kExprEnd, // end @24
]);
builder.addExport('f', 2);
const instance = builder.instantiate();
assertEquals(35, instance.exports.f(0, 0));
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