AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[Liftoff] Fix conditional spilling on div and rem

Browse Source 
On div and rem on ia32 and x64, we sometimes need to spill. If this
spilling code happens inside of a branch, the cache state will reflect
that the value was spilled, even though the actual spilling code might
not have executed.

R=titzer@chromium.org

Bug: v8:6600, chromium:839800
Change-Id: I93b681a23119f903feb54235d6d44a7cbd5815fe
Reviewed-on: https://chromium-review.googlesource.com/1044185
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52995}
This commit is contained in:
Clemens Hammacher 2018-05-04 13:54:58 +02:00 committed by Commit Bot
parent 2b4c8496d5
commit c20d7f6605
3 changed files with 28 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
src/wasm/baseline/ia32/liftoff-assembler-ia32.h
View File

@ -490,6 +490,19 @@ void EmitInt32DivOrRem(LiftoffAssembler* assm, Register dst, Register lhs,
is_signed && div_or_rem == DivOrRem::kRem;
DCHECK_EQ(needs_unrepresentable_check, trap_div_unrepresentable != nullptr);
// For division, the lhs is always taken from {edx:eax}. Thus, make sure that
// these registers are unused. If {rhs} is stored in one of them, move it to
// another temporary register.
// Do all this before any branch, such that the code is executed
// unconditionally, as the cache state will also be modified unconditionally.
liftoff::SpillRegisters(assm, eax, edx);
if (rhs == eax || rhs == edx) {
LiftoffRegList unavailable = LiftoffRegList::ForRegs(eax, edx, lhs);
Register tmp = assm->GetUnusedRegister(kGpReg, unavailable).gp();
assm->mov(tmp, rhs);
rhs = tmp;
}
// Check for division by zero.
assm->test(rhs, rhs);
assm->j(zero, trap_div_by_zero);
@ -514,17 +527,6 @@ void EmitInt32DivOrRem(LiftoffAssembler* assm, Register dst, Register lhs,
assm->bind(&do_rem);
}
// For division, the lhs is always taken from {edx:eax}. Thus, make sure that
// these registers are unused. If {rhs} is stored in one of them, move it to
// another temporary register.
liftoff::SpillRegisters(assm, eax, edx);
if (rhs == eax || rhs == edx) {
LiftoffRegList unavailable = LiftoffRegList::ForRegs(eax, edx, lhs);
Register tmp = assm->GetUnusedRegister(kGpReg, unavailable).gp();
assm->mov(tmp, rhs);
rhs = tmp;
}
// Now move {lhs} into {eax}, then zero-extend or sign-extend into {edx}, then
// do the division.
if (lhs != eax) assm->mov(eax, lhs);

24
src/wasm/baseline/x64/liftoff-assembler-x64.h
View File

@ -455,6 +455,19 @@ void EmitIntDivOrRem(LiftoffAssembler* assm, Register dst, Register lhs,
} \
} while (false)
// For division, the lhs is always taken from {edx:eax}. Thus, make sure that
// these registers are unused. If {rhs} is stored in one of them, move it to
// another temporary register.
// Do all this before any branch, such that the code is executed
// unconditionally, as the cache state will also be modified unconditionally.
liftoff::SpillRegisters(assm, rdx, rax);
if (rhs == rax || rhs == rdx) {
LiftoffRegList unavailable = LiftoffRegList::ForRegs(rax, rdx, lhs);
Register tmp = assm->GetUnusedRegister(kGpReg, unavailable).gp();
iop(mov, tmp, rhs);
rhs = tmp;
}
// Check for division by zero.
iop(test, rhs, rhs);
assm->j(zero, trap_div_by_zero);
@ -483,17 +496,6 @@ void EmitIntDivOrRem(LiftoffAssembler* assm, Register dst, Register lhs,
assm->bind(&do_rem);
}
// For division, the lhs is always taken from {edx:eax}. Thus, make sure that
// these registers are unused. If {rhs} is stored in one of them, move it to
// another temporary register.
liftoff::SpillRegisters(assm, rdx, rax);
if (rhs == rax || rhs == rdx) {
LiftoffRegList unavailable = LiftoffRegList::ForRegs(rax, rdx, lhs);
Register tmp = assm->GetUnusedRegister(kGpReg, unavailable).gp();
iop(mov, tmp, rhs);
rhs = tmp;
}
// Now move {lhs} into {eax}, then zero-extend or sign-extend into {edx}, then
// do the division.
if (lhs != rax) iop(mov, rax, lhs);

4
test/cctest/wasm/test-run-wasm.cc
View File

@ -3257,9 +3257,9 @@ void BinOpOnDifferentRegisters(
// Keep this list small, the BinOpOnDifferentRegisters test is running long
// enough already.
static constexpr int32_t kSome32BitInputs[] = {0, 1, 31, 0xff112233};
static constexpr int32_t kSome32BitInputs[] = {0, 1, -1, 31, 0xff112233};
static constexpr int64_t kSome64BitInputs[] = {
0, 1, 31, 63, 0x100000000, 0xff11223344556677};
0, 1, -1, 31, 63, 0x100000000, 0xff11223344556677};
WASM_EXEC_TEST(I32AddOnDifferentRegisters) {
BinOpOnDifferentRegisters<int32_t>(