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

[Liftoff][ia32] Avoid register overwrite on 64 bit shift

Browse Source 
On ia32, the implementation of 64-bit shifts first moved {src} into
{dst}, then {amount} into {ecx}. This fails if {dst} overlaps {amount},
because {amount} would be overwritten before being used. Just changing
the order to these two moves would also not be correct, since {src} can
contain {ecx}.
Thus, implement this via a general parallel register move, which
resolves cycles automatically.

R=titzer@chromium.org

Bug: v8:7589, v8:6600
Change-Id: I2556b9aa66a89a067372b7713dbbb3d71d2f923f
Reviewed-on: https://chromium-review.googlesource.com/981134
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52327}
This commit is contained in:
Clemens Hammacher 2018-04-03 16:17:26 +02:00 committed by Commit Bot
parent 80d587d7ea
commit c83af36f69
3 changed files with 33 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -570,32 +570,29 @@ inline void Emit64BitShiftOperation(
pinned.set(dst);
pinned.set(src);
pinned.set(amount);
// If dst contains ecx, replace it by an unused register, which is then moved
// to ecx in the end.
// If {dst} contains {ecx}, replace it by an unused register, which is then
// moved to {ecx} in the end.
Register ecx_replace = no_reg;
if (PairContains(dst, ecx)) {
ecx_replace = pinned.set(assm->GetUnusedRegister(kGpReg, pinned)).gp();
dst = ReplaceInPair(dst, ecx, ecx_replace);
// If {amount} needs to be moved to {ecx}, but {ecx} is in use (and not part
// of {dst}, hence overwritten anyway), move {ecx} to a tmp register and
// restore it at the end.
} else if (amount != ecx &&
assm->cache_state()->is_used(LiftoffRegister(ecx))) {
ecx_replace = assm->GetUnusedRegister(kGpReg, pinned).gp();
assm->mov(ecx_replace, ecx);
}
// Move src to dst.
if (dst != src) assm->Move(dst, src, kWasmI64);
// Move amount into ecx. If ecx is in use and not part of dst, move its
// content to a tmp register first.
if (amount != ecx) {
if (assm->cache_state()->is_used(LiftoffRegister(ecx)) &&
ecx_replace == no_reg) {
ecx_replace = assm->GetUnusedRegister(kGpReg, pinned).gp();
assm->mov(ecx_replace, ecx);
}
assm->mov(ecx, amount);
}
assm->ParallelRegisterMove(
{{dst, src, kWasmI64},
{LiftoffRegister{ecx}, LiftoffRegister{amount}, kWasmI32}});
// Do the actual shift.
(assm->*emit_shift)(dst.high_gp(), dst.low_gp());
// Restore ecx if needed.
// Restore {ecx} if needed.
if (ecx_replace != no_reg) assm->mov(ecx, ecx_replace);
}
} // namespace liftoff

9
src/wasm/baseline/liftoff-assembler.cc
View File

@ -577,6 +577,15 @@ void LiftoffAssembler::Move(LiftoffRegister dst, LiftoffRegister src,
}
}
void LiftoffAssembler::ParallelRegisterMove(
std::initializer_list<ParallelRegisterMoveTuple> tuples) {
StackTransferRecipe stack_transfers(this);
for (auto tuple : tuples) {
if (tuple.dst == tuple.src) continue;
stack_transfers.MoveRegister(tuple.dst, tuple.src, tuple.type);
}
}
LiftoffRegister LiftoffAssembler::SpillOneRegister(LiftoffRegList candidates,
LiftoffRegList pinned) {
// Spill one cached value to free a register.

11
src/wasm/baseline/liftoff-assembler.h
View File

@ -326,8 +326,19 @@ class LiftoffAssembler : public TurboAssembler {
// Process return values of the call.
void FinishCall(wasm::FunctionSig*, compiler::CallDescriptor*);
// Move {src} into {dst}. {src} and {dst} must be different.
void Move(LiftoffRegister dst, LiftoffRegister src, ValueType);
// Parallel register move: For a list of tuples <dst, src, type>, move the
// {src} register of type {type} into {dst}. If {src} equals {dst}, ignore
// that tuple.
struct ParallelRegisterMoveTuple {
LiftoffRegister dst;
LiftoffRegister src;
ValueType type;
};
void ParallelRegisterMove(std::initializer_list<ParallelRegisterMoveTuple>);
////////////////////////////////////
// Platform-specific part. //
////////////////////////////////////