From 491eff86b5fc0a9592a999c0889ce9c46c51618d Mon Sep 17 00:00:00 2001 From: George Wort Date: Fri, 21 Dec 2018 14:02:37 +0000 Subject: [PATCH] [liftoff][arm] GetUnusedRegister before Acquire Ensure that GetUnusedRegister is always called before acquiring the scratch register in case it is needed for spilling the value of the used register. Bug: v8:6600, chromium:910824 Change-Id: I93ae684ad504584807dfa6227b6af14609c6bcf5 Reviewed-on: https://chromium-review.googlesource.com/c/1387498 Reviewed-by: Clemens Hammacher Commit-Queue: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#58442} --- src/wasm/baseline/arm/liftoff-assembler-arm.h | 59 +++++++++---------- test/mjsunit/regress/wasm/regress-910824.js | 37 ++++++++++++ 2 files changed, 64 insertions(+), 32 deletions(-) create mode 100644 test/mjsunit/regress/wasm/regress-910824.js diff --git a/src/wasm/baseline/arm/liftoff-assembler-arm.h b/src/wasm/baseline/arm/liftoff-assembler-arm.h index 39ba5c9b1e..a54cf41733 100644 --- a/src/wasm/baseline/arm/liftoff-assembler-arm.h +++ b/src/wasm/baseline/arm/liftoff-assembler-arm.h @@ -316,12 +316,8 @@ void LiftoffAssembler::Load(LiftoffRegister dst, Register src_addr, } else { // TODO(arm): Use vld1 for f32 when implemented in simulator as used for // f64. It supports unaligned access. - Register scratch = no_reg; - if (temps.CanAcquire()) { - scratch = temps.Acquire(); - } else { - scratch = GetUnusedRegister(kGpReg, pinned).gp(); - } + Register scratch = + (actual_src_addr == src_addr) ? temps.Acquire() : actual_src_addr; ldr(scratch, MemOperand(actual_src_addr)); vmov(liftoff::GetFloatRegister(dst.fp()), scratch); } @@ -400,26 +396,25 @@ void LiftoffAssembler::Store(Register dst_addr, Register offset_reg, return; } UseScratchRegisterScope temps(this); - if (type.value() == StoreType::kF64Store || - type.value() == StoreType::kF32Store) { + if (type.value() == StoreType::kF64Store) { Register actual_dst_addr = liftoff::CalculateActualAddress( this, &temps, dst_addr, offset_reg, offset_imm); - if (type.value() == StoreType::kF64Store) { - // Armv6 is not supported so Neon can be used to avoid alignment issues. - CpuFeatureScope scope(this, NEON); - vst1(Neon64, NeonListOperand(src.fp()), NeonMemOperand(actual_dst_addr)); - } else { - // TODO(arm): Use vst1 for f32 when implemented in simulator as used for - // f64. It supports unaligned access. - Register scratch = no_reg; - if (temps.CanAcquire()) { - scratch = temps.Acquire(); - } else { - scratch = GetUnusedRegister(kGpReg, pinned).gp(); - } - vmov(scratch, liftoff::GetFloatRegister(src.fp())); - str(scratch, MemOperand(actual_dst_addr)); - } + // Armv6 is not supported so Neon can be used to avoid alignment issues. + CpuFeatureScope scope(this, NEON); + vst1(Neon64, NeonListOperand(src.fp()), NeonMemOperand(actual_dst_addr)); + } else if (type.value() == StoreType::kF32Store) { + // TODO(arm): Use vst1 for f32 when implemented in simulator as used for + // f64. It supports unaligned access. + // CalculateActualAddress will only not use a scratch register if the + // following condition holds, otherwise another register must be + // retrieved. + Register scratch = (offset_reg == no_reg && offset_imm == 0) + ? temps.Acquire() + : GetUnusedRegister(kGpReg, pinned).gp(); + Register actual_dst_addr = liftoff::CalculateActualAddress( + this, &temps, dst_addr, offset_reg, offset_imm); + vmov(scratch, liftoff::GetFloatRegister(src.fp())); + str(scratch, MemOperand(actual_dst_addr)); } else { MemOperand dst_op = liftoff::GetMemOp(this, &temps, dst_addr, offset_reg, offset_imm); @@ -671,13 +666,13 @@ bool LiftoffAssembler::emit_i32_ctz(Register dst, Register src) { bool LiftoffAssembler::emit_i32_popcnt(Register dst, Register src) { { UseScratchRegisterScope temps(this); - Register scratch = temps.Acquire(); + LiftoffRegList pinned; + pinned.set(dst); + Register scratch = GetUnusedRegister(kGpReg, pinned).gp(); + Register scratch_2 = temps.Acquire(); // x = x - ((x & (0x55555555 << 1)) >> 1) and_(scratch, src, Operand(0xaaaaaaaa)); sub(dst, src, Operand(scratch, LSR, 1)); - LiftoffRegList pinned; - pinned.set(dst); - Register scratch_2 = GetUnusedRegister(kGpReg, pinned).gp(); // x = (x & 0x33333333) + ((x & (0x33333333 << 2)) >> 2) mov(scratch, Operand(0x33333333)); and_(scratch_2, dst, Operand(scratch, LSL, 2)); @@ -970,8 +965,8 @@ void LiftoffAssembler::emit_f32_copysign(DoubleRegister dst, DoubleRegister lhs, DoubleRegister rhs) { constexpr uint32_t kF32SignBit = uint32_t{1} << 31; UseScratchRegisterScope temps(this); - Register scratch = temps.Acquire(); - Register scratch2 = GetUnusedRegister(kGpReg).gp(); + Register scratch = GetUnusedRegister(kGpReg).gp(); + Register scratch2 = temps.Acquire(); VmovLow(scratch, lhs); // Clear sign bit in {scratch}. bic(scratch, scratch, Operand(kF32SignBit)); @@ -989,8 +984,8 @@ void LiftoffAssembler::emit_f64_copysign(DoubleRegister dst, DoubleRegister lhs, // On arm, we cannot hold the whole f64 value in a gp register, so we just // operate on the upper half (UH). UseScratchRegisterScope temps(this); - Register scratch = temps.Acquire(); - Register scratch2 = GetUnusedRegister(kGpReg).gp(); + Register scratch = GetUnusedRegister(kGpReg).gp(); + Register scratch2 = temps.Acquire(); VmovHigh(scratch, lhs); // Clear sign bit in {scratch}. bic(scratch, scratch, Operand(kF64SignBitHighWord)); diff --git a/test/mjsunit/regress/wasm/regress-910824.js b/test/mjsunit/regress/wasm/regress-910824.js new file mode 100644 index 0000000000..7c8f154496 --- /dev/null +++ b/test/mjsunit/regress/wasm/regress-910824.js @@ -0,0 +1,37 @@ +// Copyright 2018 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-constants.js'); +load('test/mjsunit/wasm/wasm-module-builder.js'); + +const builder = new WasmModuleBuilder(); +builder.addGlobal(kWasmI32, 1); +builder.addGlobal(kWasmF32, 1); +builder.addType(makeSig([kWasmI32, kWasmF32, kWasmF32, kWasmF64], [kWasmI32])); +builder.addFunction(undefined, 0 /* sig */) + .addLocals({i32_count: 504}) + .addBody([ +kExprGetGlobal, 0x00, +kExprSetLocal, 0x04, +kExprGetLocal, 0x04, +kExprI32Const, 0x01, +kExprI32Sub, +kExprGetGlobal, 0x00, +kExprI32Const, 0x00, +kExprI32Eqz, +kExprGetGlobal, 0x00, +kExprI32Const, 0x01, +kExprI32Const, 0x01, +kExprI32Sub, +kExprGetGlobal, 0x00, +kExprI32Const, 0x00, +kExprI32Eqz, +kExprGetGlobal, 0x00, +kExprI32Const, 0x00, +kExprI32Const, 0x01, +kExprI32Sub, +kExprGetGlobal, 0x01, +kExprUnreachable, +]); +builder.instantiate();