cherry-pick-8c725f7b5bbf.patch 5.2 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127
  1. From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
  2. From: Thibaud Michaud <[email protected]>
  3. Date: Thu, 15 Oct 2020 12:45:34 +0200
  4. Subject: Merged: [codegen] Skip invalid optimization in tail calls
  5. MIME-Version: 1.0
  6. Content-Type: text/plain; charset=UTF-8
  7. Content-Transfer-Encoding: 8bit
  8. Preparing for tail call is usually done by emitting the gap moves and
  9. then moving the stack pointer to its new position. An optimization
  10. consists in moving the stack pointer first and transforming some of the
  11. moves into pushes. In the attached case it looks like this (arm):
  12. 138 add sp, sp, #40
  13. 13c str r6, [sp, #-4]!
  14. 140 str r6, [sp, #-4]!
  15. 144 str r6, [sp, #-4]!
  16. 148 str r6, [sp, #-4]!
  17. 14c str r6, [sp, #-4]!
  18. ...
  19. 160 vldr d1, [sp - 4*3]
  20. The last line is a gap reload, but because the stack pointer was already
  21. moved, the slot is now below the stack pointer. This is invalid and
  22. triggers this DCHECK:
  23. Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
  24. Debug check failed: 0 <= offset (0 vs. -12).
  25. A comment already explains that we skip the optimization if the gap
  26. contains stack moves to prevent this, but the code only checks for
  27. non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
  28. "source.IsAnyStackSlot()":
  29. 108 vldr d1, [sp + 4*2]
  30. ...
  31. 118 str r0, [sp, #+36]
  32. 11c str r0, [sp, #+32]
  33. 120 str r0, [sp, #+28]
  34. 124 str r0, [sp, #+24]
  35. 128 str r0, [sp, #+20]
  36. ...
  37. 134 add sp, sp, #20
  38. TBR=​[email protected]
  39. (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)
  40. Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
  41. Bug: chromium:1137608
  42. No-Try: true
  43. No-Presubmit: true
  44. No-Tree-Checks: true
  45. Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
  46. Reviewed-by: Thibaud Michaud <[email protected]>
  47. Commit-Queue: Thibaud Michaud <[email protected]>
  48. Cr-Commit-Position: refs/branch-heads/8.6@{#42}
  49. Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
  50. Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}
  51. diff --git a/src/compiler/backend/code-generator.cc b/src/compiler/backend/code-generator.cc
  52. index 36231c61e09839b3394b78b4f4715daa3023caf4..c0162012eb2747fb6b64c35f336198d0bf75a4ab 100644
  53. --- a/src/compiler/backend/code-generator.cc
  54. +++ b/src/compiler/backend/code-generator.cc
  55. @@ -564,8 +564,8 @@ void CodeGenerator::GetPushCompatibleMoves(Instruction* instr,
  56. // then the full gap resolver must be used since optimization with
  57. // pushes don't participate in the parallel move and might clobber
  58. // values needed for the gap resolve.
  59. - if (source.IsStackSlot() && LocationOperand::cast(source).index() >=
  60. - first_push_compatible_index) {
  61. + if (source.IsAnyStackSlot() && LocationOperand::cast(source).index() >=
  62. + first_push_compatible_index) {
  63. pushes->clear();
  64. return;
  65. }
  66. diff --git a/test/mjsunit/regress/wasm/regress-1137608.js b/test/mjsunit/regress/wasm/regress-1137608.js
  67. new file mode 100644
  68. index 0000000000000000000000000000000000000000..5011dced2f70ffbe2b6096a4e1863f434c8898a8
  69. --- /dev/null
  70. +++ b/test/mjsunit/regress/wasm/regress-1137608.js
  71. @@ -0,0 +1,46 @@
  72. +// Copyright 2020 the V8 project authors. All rights reserved.
  73. +// Use of this source code is governed by a BSD-style license that can be
  74. +// found in the LICENSE file.
  75. +//
  76. +// Flags: --no-liftoff --experimental-wasm-return-call --experimental-wasm-threads
  77. +
  78. +load("test/mjsunit/wasm/wasm-module-builder.js");
  79. +
  80. +(function Regress1137608() {
  81. + print(arguments.callee.name);
  82. + let builder = new WasmModuleBuilder();
  83. + let sig0 = builder.addType(kSig_i_iii);
  84. + let sig1 = builder.addType(makeSig([kWasmF64, kWasmF64, kWasmI32,
  85. + kWasmI32, kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32, kWasmF32,
  86. + kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32], [kWasmI32]));
  87. + let main = builder.addFunction("main", sig0)
  88. + .addBody([
  89. + kExprI64Const, 0,
  90. + kExprF64UConvertI64,
  91. + kExprF64Const, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x00, 0x00,
  92. + kExprF64Const, 0x30, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00,
  93. + kExprF64Mul,
  94. + kExprI32Const, 0,
  95. + kExprF64Const, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  96. + kExprF64StoreMem, 0x00, 0xb0, 0xe0, 0xc0, 0x81, 0x03,
  97. + kExprI32Const, 0,
  98. + kExprI32Const, 0,
  99. + kExprI32Const, 0,
  100. + kExprF32Const, 0x00, 0x00, 0x00, 0x00,
  101. + kExprI32Const, 0,
  102. + kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  103. + kExprI32Const, 0,
  104. + kExprF32Const, 0x00, 0x00, 0x00, 0x00,
  105. + kExprI32Const, 0,
  106. + kExprF32Const, 0x00, 0x00, 0x00, 0x00,
  107. + kExprI32Const, 0,
  108. + kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
  109. + kExprI32Const, 0,
  110. + kExprI32Const, 2,
  111. + kExprReturnCallIndirect, sig1, kTableZero]).exportFunc();
  112. + builder.addFunction("f", sig1).addBody([kExprI32Const, 0]);
  113. + builder.addTable(kWasmAnyFunc, 4, 4);
  114. + builder.addMemory(16, 32, false, true);
  115. + let module = new WebAssembly.Module(builder.toBuffer());
  116. + let instance = new WebAssembly.Instance(module);
  117. +})();