cherry-pick-1231950.patch 8.5 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: Clemens Backes <[email protected]>
  3. Date: Sat, 24 Jul 2021 09:07:34 +0200
  4. Subject: Reland "[liftoff][arm64] Zero-extend offsets also for SIMD"
  5. MIME-Version: 1.0
  6. Content-Type: text/plain; charset=UTF-8
  7. Content-Transfer-Encoding: 8bit
  8. This is a reland of b99fe75c6db86d86ad8989458d28978b001d9234.
  9. The test is now skipped on non-SIMD hardware.
  10. Original change's description:
  11. > [liftoff][arm64] Zero-extend offsets also for SIMD
  12. >
  13. > This extends https://crrev.com/c/2917612 also for SIMD, which
  14. > (sometimes) uses the special {GetMemOpWithImmOffsetZero} method.
  15. > As part of this CL, that method is renamed to {GetEffectiveAddress}
  16. > which IMO is a better name. Also, it just returns a register to make the
  17. > semantic of that function obvious in the signature.
  18. >
  19. > Drive-by: When sign extending to 32 bit, only write to the W portion of
  20. > the register. This is a bit cleaner, and I first thought that
  21. > this would be the bug.
  22. >
  23. > [email protected]
  24. > CC=​[email protected]
  25. >
  26. > Bug: chromium:1231950, v8:12018
  27. > Change-Id: Ifaefe1f18e3a00534a30c99e3c37ed09d9508f6e
  28. > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3049073
  29. > Reviewed-by: Zhi An Ng <[email protected]>
  30. > Commit-Queue: Clemens Backes <[email protected]>
  31. > Cr-Commit-Position: refs/heads/master@{#75898}
  32. [email protected]
  33. (cherry picked from commit 5e90a612f56109f611b7e32117f41b10a845186e)
  34. Bug: chromium:1231950
  35. Change-Id: I33916616bb736cd254ebf121463257a0584bf513
  36. No-Try: true
  37. No-Tree-Checks: true
  38. No-Presubmit: true
  39. Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3059694
  40. Reviewed-by: Clemens Backes <[email protected]>
  41. Commit-Queue: Clemens Backes <[email protected]>
  42. Cr-Commit-Position: refs/branch-heads/9.2@{#43}
  43. Cr-Branched-From: 51238348f95a1f5e0acc321efac7942d18a687a2-refs/heads/9.2.230@{#1}
  44. Cr-Branched-From: 587a04f02ab0487d194b55a7137dc2045e071597-refs/heads/master@{#74656}
  45. diff --git a/src/wasm/baseline/arm64/liftoff-assembler-arm64.h b/src/wasm/baseline/arm64/liftoff-assembler-arm64.h
  46. index 39ef8528e5267a76f69a251d358ed5a9259246e0..17b2b840f236c249e5a9d2a5fa61098ba3c9f35a 100644
  47. --- a/src/wasm/baseline/arm64/liftoff-assembler-arm64.h
  48. +++ b/src/wasm/baseline/arm64/liftoff-assembler-arm64.h
  49. @@ -137,27 +137,23 @@ inline MemOperand GetMemOp(LiftoffAssembler* assm,
  50. return MemOperand(addr.X(), offset_imm);
  51. }
  52. -// Certain load instructions do not support offset (register or immediate).
  53. -// This creates a MemOperand that is suitable for such instructions by adding
  54. -// |addr|, |offset| (if needed), and |offset_imm| into a temporary.
  55. -inline MemOperand GetMemOpWithImmOffsetZero(LiftoffAssembler* assm,
  56. - UseScratchRegisterScope* temps,
  57. - Register addr, Register offset,
  58. - uintptr_t offset_imm) {
  59. +// Compute the effective address (sum of |addr|, |offset| (if given) and
  60. +// |offset_imm|) into a temporary register. This is needed for certain load
  61. +// instructions that do not support an offset (register or immediate).
  62. +// Returns |addr| if both |offset| and |offset_imm| are zero.
  63. +inline Register GetEffectiveAddress(LiftoffAssembler* assm,
  64. + UseScratchRegisterScope* temps,
  65. + Register addr, Register offset,
  66. + uintptr_t offset_imm) {
  67. + if (!offset.is_valid() && offset_imm == 0) return addr;
  68. Register tmp = temps->AcquireX();
  69. if (offset.is_valid()) {
  70. - // offset has passed BoundsCheckMem in liftoff-compiler, and been unsigned
  71. - // extended, so it is fine to use the full width of the register.
  72. - assm->Add(tmp, addr, offset);
  73. - if (offset_imm != 0) {
  74. - assm->Add(tmp, tmp, offset_imm);
  75. - }
  76. - } else {
  77. - if (offset_imm != 0) {
  78. - assm->Add(tmp, addr, offset_imm);
  79. - }
  80. + // TODO(clemensb): This needs adaption for memory64.
  81. + assm->Add(tmp, addr, Operand(offset, UXTW));
  82. + addr = tmp;
  83. }
  84. - return MemOperand(tmp.X(), 0);
  85. + if (offset_imm != 0) assm->Add(tmp, addr, offset_imm);
  86. + return tmp;
  87. }
  88. enum class ShiftDirection : bool { kLeft, kRight };
  89. @@ -1491,11 +1487,11 @@ bool LiftoffAssembler::emit_type_conversion(WasmOpcode opcode,
  90. }
  91. void LiftoffAssembler::emit_i32_signextend_i8(Register dst, Register src) {
  92. - sxtb(dst, src);
  93. + sxtb(dst.W(), src.W());
  94. }
  95. void LiftoffAssembler::emit_i32_signextend_i16(Register dst, Register src) {
  96. - sxth(dst, src);
  97. + sxth(dst.W(), src.W());
  98. }
  99. void LiftoffAssembler::emit_i64_signextend_i8(LiftoffRegister dst,
  100. @@ -1629,8 +1625,8 @@ void LiftoffAssembler::LoadTransform(LiftoffRegister dst, Register src_addr,
  101. UseScratchRegisterScope temps(this);
  102. MemOperand src_op =
  103. transform == LoadTransformationKind::kSplat
  104. - ? liftoff::GetMemOpWithImmOffsetZero(this, &temps, src_addr,
  105. - offset_reg, offset_imm)
  106. + ? MemOperand{liftoff::GetEffectiveAddress(this, &temps, src_addr,
  107. + offset_reg, offset_imm)}
  108. : liftoff::GetMemOp(this, &temps, src_addr, offset_reg, offset_imm);
  109. *protected_load_pc = pc_offset();
  110. MachineType memtype = type.mem_type();
  111. @@ -1681,8 +1677,8 @@ void LiftoffAssembler::LoadLane(LiftoffRegister dst, LiftoffRegister src,
  112. uintptr_t offset_imm, LoadType type,
  113. uint8_t laneidx, uint32_t* protected_load_pc) {
  114. UseScratchRegisterScope temps(this);
  115. - MemOperand src_op = liftoff::GetMemOpWithImmOffsetZero(
  116. - this, &temps, addr, offset_reg, offset_imm);
  117. + MemOperand src_op{
  118. + liftoff::GetEffectiveAddress(this, &temps, addr, offset_reg, offset_imm)};
  119. *protected_load_pc = pc_offset();
  120. MachineType mem_type = type.mem_type();
  121. @@ -1708,8 +1704,8 @@ void LiftoffAssembler::StoreLane(Register dst, Register offset,
  122. StoreType type, uint8_t lane,
  123. uint32_t* protected_store_pc) {
  124. UseScratchRegisterScope temps(this);
  125. - MemOperand dst_op =
  126. - liftoff::GetMemOpWithImmOffsetZero(this, &temps, dst, offset, offset_imm);
  127. + MemOperand dst_op{
  128. + liftoff::GetEffectiveAddress(this, &temps, dst, offset, offset_imm)};
  129. if (protected_store_pc) *protected_store_pc = pc_offset();
  130. MachineRepresentation rep = type.mem_rep();
  131. diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
  132. index b6dd59ec697547fa5c56c2ecdb679186ca7f80c7..bb860434536ebc2d5296c5bbf979a4f2f87bc6dd 100644
  133. --- a/test/mjsunit/mjsunit.status
  134. +++ b/test/mjsunit/mjsunit.status
  135. @@ -1447,7 +1447,8 @@
  136. 'regress/wasm/regress-1161954': [SKIP],
  137. 'regress/wasm/regress-1165966': [SKIP],
  138. 'regress/wasm/regress-1187831': [SKIP],
  139. -}], # no_simd_sse == True
  140. + 'regress/wasm/regress-1231950': [SKIP],
  141. +}], # no_simd_hardware == True
  142. ##############################################################################
  143. # TODO(v8:11421): Port baseline compiler to other architectures.
  144. @@ -1465,4 +1466,10 @@
  145. 'concurrent-initial-prototype-change-1': [SKIP],
  146. }], # variant == concurrent_inlining
  147. +##############################################################################
  148. +['variant == instruction_scheduling or variant == stress_instruction_scheduling', {
  149. + # BUG(12018): This test currently fails with --turbo-instruction-scheduling.
  150. + 'regress/wasm/regress-1231950': [SKIP],
  151. +}], # variant == instruction_scheduling or variant == stress_instruction_scheduling
  152. +
  153. ]
  154. diff --git a/test/mjsunit/regress/wasm/regress-1231950.js b/test/mjsunit/regress/wasm/regress-1231950.js
  155. new file mode 100644
  156. index 0000000000000000000000000000000000000000..972754c6d52094727a93dae4c0847f013b6c7675
  157. --- /dev/null
  158. +++ b/test/mjsunit/regress/wasm/regress-1231950.js
  159. @@ -0,0 +1,18 @@
  160. +// Copyright 2021 the V8 project authors. All rights reserved.
  161. +// Use of this source code is governed by a BSD-style license that can be
  162. +// found in the LICENSE file.
  163. +
  164. +load('test/mjsunit/wasm/wasm-module-builder.js');
  165. +
  166. +const builder = new WasmModuleBuilder();
  167. +builder.addMemory(1, 1);
  168. +builder.addFunction('main', kSig_d_v)
  169. + .addBody([
  170. + ...wasmI32Const(-3), // i32.const
  171. + kExprI32SExtendI8, // i32.extend8_s
  172. + kSimdPrefix, kExprS128Load32Splat, 0x01, 0x02, // s128.load32_splat
  173. + kExprUnreachable, // unreachable
  174. + ])
  175. + .exportFunc();
  176. +const instance = builder.instantiate();
  177. +assertTraps(kTrapMemOutOfBounds, instance.exports.main);