Skip to content

Commit d85b52b

Browse files
committed
deps: V8: cherry-pick eddb82330975
Original commit message: Merged: [wasm][liftoff] Fix register usage for i64_addi The arm implementation made the assumption that the {lhs} and {dst} registers are either the same, or there is no overlap. This assumption does not hold. ia32 on the other hand has a lot of complicated logic (and unnecessary code generation) for different cases of overlap. This CL fixes the arm issue *and* simplifies the ia32 logic by making the arm assumption hold, and using it to eliminate special handling on ia32. R=​[email protected] (cherry picked from commit 89ca48c907e25ef94a135255092c4e150654c4fc) Bug: chromium:1146861 Change-Id: I96c4985fb8ff710b98e009e457444fc8804bce58 No-Try: true No-Presubmit: true No-Tree-Checks: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2584242 Reviewed-by: Thibaud Michaud <[email protected]> Commit-Queue: Clemens Backes <[email protected]> Cr-Commit-Position: refs/branch-heads/8.6@{nodejs#50} Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1} Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472} Refs: v8/v8@eddb823
1 parent 2b8101b commit d85b52b

File tree

5 files changed

+73
-24
lines changed

5 files changed

+73
-24
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.58',
39+
'v8_embedder_string': '-node.59',
4040

4141
##### V8 defaults for Node.js #####
4242

deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h

+2
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ template <void (Assembler::*op)(Register, Register, const Operand&, SBit,
140140
SBit, Condition)>
141141
inline void I64BinopI(LiftoffAssembler* assm, LiftoffRegister dst,
142142
LiftoffRegister lhs, int32_t imm) {
143+
// The compiler allocated registers such that either {dst == lhs} or there is
144+
// no overlap between the two.
143145
DCHECK_NE(dst.low_gp(), lhs.high_gp());
144146
(assm->*op)(dst.low_gp(), lhs.low_gp(), Operand(imm), SetCC, al);
145147
// Top half of the immediate sign extended, either 0 or -1.

deps/v8/src/wasm/baseline/ia32/liftoff-assembler-ia32.h

+9-21
Original file line numberDiff line numberDiff line change
@@ -1091,31 +1091,19 @@ template <void (Assembler::*op)(Register, const Immediate&),
10911091
void (Assembler::*op_with_carry)(Register, int32_t)>
10921092
inline void OpWithCarryI(LiftoffAssembler* assm, LiftoffRegister dst,
10931093
LiftoffRegister lhs, int32_t imm) {
1094-
// First, compute the low half of the result, potentially into a temporary dst
1095-
// register if {dst.low_gp()} equals any register we need to
1096-
// keep alive for computing the upper half.
1097-
LiftoffRegList keep_alive = LiftoffRegList::ForRegs(lhs.high_gp());
1098-
Register dst_low = keep_alive.has(dst.low_gp())
1099-
? assm->GetUnusedRegister(kGpReg, keep_alive).gp()
1100-
: dst.low_gp();
1101-
1102-
if (dst_low != lhs.low_gp()) assm->mov(dst_low, lhs.low_gp());
1103-
(assm->*op)(dst_low, Immediate(imm));
1094+
// The compiler allocated registers such that either {dst == lhs} or there is
1095+
// no overlap between the two.
1096+
DCHECK_NE(dst.low_gp(), lhs.high_gp());
11041097

1105-
// Now compute the upper half, while keeping alive the previous result.
1106-
keep_alive = LiftoffRegList::ForRegs(dst_low);
1107-
Register dst_high = keep_alive.has(dst.high_gp())
1108-
? assm->GetUnusedRegister(kGpReg, keep_alive).gp()
1109-
: dst.high_gp();
1098+
// First, compute the low half of the result.
1099+
if (dst.low_gp() != lhs.low_gp()) assm->mov(dst.low_gp(), lhs.low_gp());
1100+
(assm->*op)(dst.low_gp(), Immediate(imm));
11101101

1111-
if (dst_high != lhs.high_gp()) assm->mov(dst_high, lhs.high_gp());
1102+
// Now compute the upper half.
1103+
if (dst.high_gp() != lhs.high_gp()) assm->mov(dst.high_gp(), lhs.high_gp());
11121104
// Top half of the immediate sign extended, either 0 or -1.
11131105
int32_t sign_extend = imm < 0 ? -1 : 0;
1114-
(assm->*op_with_carry)(dst_high, sign_extend);
1115-
1116-
// If necessary, move result into the right registers.
1117-
LiftoffRegister tmp_result = LiftoffRegister::ForPair(dst_low, dst_high);
1118-
if (tmp_result != dst) assm->Move(dst, tmp_result, kWasmI64);
1106+
(assm->*op_with_carry)(dst.high_gp(), sign_extend);
11191107
}
11201108
} // namespace liftoff
11211109

deps/v8/src/wasm/baseline/liftoff-compiler.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -1122,9 +1122,12 @@ class LiftoffCompiler {
11221122
int32_t imm = rhs_slot.i32_const();
11231123

11241124
LiftoffRegister lhs = __ PopToRegister();
1125+
// Either reuse {lhs} for {dst}, or choose a register (pair) which does
1126+
// not overlap, for easier code generation.
1127+
LiftoffRegList pinned = LiftoffRegList::ForRegs(lhs);
11251128
LiftoffRegister dst = src_rc == result_rc
1126-
? __ GetUnusedRegister(result_rc, {lhs}, {})
1127-
: __ GetUnusedRegister(result_rc, {});
1129+
? __ GetUnusedRegister(result_rc, {lhs}, pinned)
1130+
: __ GetUnusedRegister(result_rc, pinned);
11281131

11291132
CallEmitFn(fnImm, dst, lhs, imm);
11301133
__ PushRegister(ValueType(result_type), dst);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright 2020 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
load('test/mjsunit/wasm/wasm-module-builder.js');
6+
7+
const builder = new WasmModuleBuilder();
8+
builder.addGlobal(kWasmI32, 1);
9+
builder.addType(makeSig([], [kWasmF64]));
10+
// Generate function 1 (out of 1).
11+
builder.addFunction(undefined, 0 /* sig */)
12+
.addLocals(kWasmI32, 8).addLocals(kWasmI64, 3)
13+
.addBodyWithEnd([
14+
// signature: d_v
15+
// body:
16+
kExprGlobalGet, 0x00, // global.get
17+
kExprLocalSet, 0x00, // local.set
18+
kExprI32Const, 0x00, // i32.const
19+
kExprI32Eqz, // i32.eqz
20+
kExprLocalSet, 0x01, // local.set
21+
kExprGlobalGet, 0x00, // global.get
22+
kExprLocalSet, 0x02, // local.set
23+
kExprI32Const, 0x01, // i32.const
24+
kExprI32Const, 0x01, // i32.const
25+
kExprI32Sub, // i32.sub
26+
kExprLocalSet, 0x03, // local.set
27+
kExprGlobalGet, 0x00, // global.get
28+
kExprLocalSet, 0x04, // local.set
29+
kExprI32Const, 0x00, // i32.const
30+
kExprI32Eqz, // i32.eqz
31+
kExprLocalSet, 0x05, // local.set
32+
kExprGlobalGet, 0x00, // global.get
33+
kExprLocalSet, 0x06, // local.set
34+
kExprI32Const, 0x00, // i32.const
35+
kExprI32Const, 0x01, // i32.const
36+
kExprI32Sub, // i32.sub
37+
kExprLocalSet, 0x07, // local.set
38+
kExprBlock, kWasmStmt, // block @45
39+
kExprI32Const, 0x00, // i32.const
40+
kExprIf, kWasmStmt, // if @49
41+
kExprLocalGet, 0x0a, // local.get
42+
kExprLocalSet, 0x08, // local.set
43+
kExprElse, // else @55
44+
kExprNop, // nop
45+
kExprEnd, // end @57
46+
kExprLocalGet, 0x08, // local.get
47+
kExprLocalSet, 0x09, // local.set
48+
kExprLocalGet, 0x09, // local.get
49+
kExprI64Const, 0xff, 0x01, // i64.const
50+
kExprI64Add, // i64.add
51+
kExprDrop, // drop
52+
kExprEnd, // end @69
53+
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xf0, 0x3f, // f64.const
54+
kExprEnd, // end @79
55+
]);
56+
builder.instantiate();

0 commit comments

Comments
 (0)