Skip to content

Commit

Permalink
[liftoff] Handle unordered register pairs
Browse files Browse the repository at this point in the history
For 64-bit binary operations, Liftoff on arm made the assumption that
register pairs are always ordered, i.e. the register code for the low
word is lower than the register code for the high word.
Ensuring this was only implemented in {GetUnusedRegister} in
https://crrev.com/c/2168875. Other cases were missing though, e.g.
return values, but also different places were we
construct register pairs internally.

Thus, this CL removes this constraint again and instead handles
unordered register pairs in 64-bit binary operations on arm.

R=thibaudm@chromium.org

Bug: chromium:1101304
Change-Id: I4cd9fb1577f82ab06d34c9dde6533cf04a2cade7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2287870
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68752}
  • Loading branch information
backes authored and Commit Bot committed Jul 9, 2020
1 parent d6a14ab commit b429b8f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
11 changes: 8 additions & 3 deletions src/wasm/baseline/arm/liftoff-assembler-arm.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,16 @@ template <void (Assembler::*op)(Register, Register, Register, SBit, Condition),
SBit, Condition)>
inline void I64Binop(LiftoffAssembler* assm, LiftoffRegister dst,
LiftoffRegister lhs, LiftoffRegister rhs) {
DCHECK_NE(dst.low_gp(), lhs.high_gp());
DCHECK_NE(dst.low_gp(), rhs.high_gp());
(assm->*op)(dst.low_gp(), lhs.low_gp(), rhs.low_gp(), SetCC, al);
Register dst_low = dst.low_gp();
if (dst_low == lhs.high_gp() || dst_low == rhs.high_gp()) {
dst_low = assm->GetUnusedRegister(
kGpReg, LiftoffRegList::ForRegs(lhs, rhs, dst.high_gp()))
.gp();
}
(assm->*op)(dst_low, lhs.low_gp(), rhs.low_gp(), SetCC, al);
(assm->*op_with_carry)(dst.high_gp(), lhs.high_gp(), Operand(rhs.high_gp()),
LeaveCC, al);
if (dst_low != dst.low_gp()) assm->mov(dst.low_gp(), dst_low);
}

template <void (Assembler::*op)(Register, Register, const Operand&, SBit,
Expand Down
9 changes: 0 additions & 9 deletions src/wasm/baseline/liftoff-assembler.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,15 +371,6 @@ class LiftoffAssembler : public TurboAssembler {
LiftoffRegList candidates = kGpCacheRegList;
Register low = pinned.set(GetUnusedRegister(candidates, pinned)).gp();
Register high = GetUnusedRegister(candidates, pinned).gp();
if (low.code() > high.code()) {
// Establish the invariant that the register of the low word always has
// a lower code than the register of the high word. This guarantees that
// if a register pair of an input is reused for the result, the low
// word and high word registers are not swapped, i.e. the low word
// register of the result is not the high word register of the input,
// and vice versa.
std::swap(low, high);
}
return LiftoffRegister::ForPair(low, high);
} else if (kNeedS128RegPair && rc == kFpRegPair) {
// kFpRegPair specific logic here because we need adjacent registers, not
Expand Down
29 changes: 29 additions & 0 deletions test/mjsunit/regress/wasm/regress-1101304.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// 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.addType(makeSig(
[kWasmI64, kWasmI32, kWasmF64, kWasmI64, kWasmI64, kWasmI32, kWasmI64],
[]));
builder.addFunction(undefined, 0 /* sig */).addBody([
kExprI32Const, 0, // i32.const
kExprIf, kWasmStmt, // if @3
kExprI32Const, 1, // i32.const
kExprIf, kWasmStmt, // if @7
kExprNop, // nop
kExprElse, // else @10
kExprUnreachable, // unreachable
kExprEnd, // end @12
kExprLocalGet, 0x06, // local.get
kExprLocalSet, 0x00, // local.set
kExprLocalGet, 0x03, // local.get
kExprLocalGet, 0x00, // local.get
kExprI64Sub, // i64.sub
kExprDrop, // drop
kExprUnreachable, // unreachable
kExprEnd // end @24
]);
builder.toModule();

0 comments on commit b429b8f

Please sign in to comment.