Skip to content

Commit

Permalink
deps: V8: cherry-pick 8c725f7b5bbf
Browse files Browse the repository at this point in the history
Original commit message:

    Merged: [codegen] Skip invalid optimization in tail calls

    Preparing for tail call is usually done by emitting the gap moves and
    then moving the stack pointer to its new position. An optimization
    consists in moving the stack pointer first and transforming some of the
    moves into pushes. In the attached case it looks like this (arm):

    138  add sp, sp, #40
    13c  str r6, [sp, #-4]!
    140  str r6, [sp, #-4]!
    144  str r6, [sp, #-4]!
    148  str r6, [sp, #-4]!
    14c  str r6, [sp, #-4]!
    ...
    160  vldr d1, [sp - 4*3]

    The last line is a gap reload, but because the stack pointer was already
    moved, the slot is now below the stack pointer. This is invalid and
    triggers this DCHECK:

    Fatal error in ../../v8/src/codegen/arm/assembler-arm.cc, line 402
    Debug check failed: 0 <= offset (0 vs. -12).

    A comment already explains that we skip the optimization if the gap
    contains stack moves to prevent this, but the code only checks for
    non-FP slots. This is fixed by replacing "source.IsStackSlot()" with
    "source.IsAnyStackSlot()":

    108  vldr d1, [sp + 4*2]
    ...
    118  str r0, [sp, #+36]
    11c  str r0, [sp, #+32]
    120  str r0, [sp, #+28]
    124  str r0, [sp, #+24]
    128  str r0, [sp, #+20]
    ...
    134  add sp, sp, #20

    TBR=​[email protected]

    (cherry picked from commit 7506e063d0d7fb00e4b9c06735c91e1953296867)

    Change-Id: I66ed6187755af956e245207e940c83ea0697a5e6
    Bug: chromium:1137608
    No-Try: true
    No-Presubmit: true
    No-Tree-Checks: true
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2505976
    Reviewed-by: Thibaud Michaud <[email protected]>
    Commit-Queue: Thibaud Michaud <[email protected]>
    Cr-Commit-Position: refs/branch-heads/8.6@{#42}
    Cr-Branched-From: a64aed2333abf49e494d2a5ce24bbd14fff19f60-refs/heads/8.6.395@{#1}
    Cr-Branched-From: a626bc036236c9bf92ac7b87dc40c9e538b087e3-refs/heads/master@{#69472}

Refs: v8/v8@8c725f7

PR-URL: #38275
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
  • Loading branch information
targos committed Apr 30, 2021
1 parent 0e6976f commit 70f622b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.36',
'v8_embedder_string': '-node.37',

##### V8 defaults for Node.js #####

Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/compiler/backend/code-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,8 @@ void CodeGenerator::GetPushCompatibleMoves(Instruction* instr,
// then the full gap resolver must be used since optimization with
// pushes don't participate in the parallel move and might clobber
// values needed for the gap resolve.
if (source.IsStackSlot() && LocationOperand::cast(source).index() >=
first_push_compatible_index) {
if (source.IsAnyStackSlot() && LocationOperand::cast(source).index() >=
first_push_compatible_index) {
pushes->clear();
return;
}
Expand Down
46 changes: 46 additions & 0 deletions deps/v8/test/mjsunit/regress/wasm/regress-1137608.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2020 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.
//
// Flags: --no-liftoff --experimental-wasm-return-call --experimental-wasm-threads

load("test/mjsunit/wasm/wasm-module-builder.js");

(function Regress1137608() {
print(arguments.callee.name);
let builder = new WasmModuleBuilder();
let sig0 = builder.addType(kSig_i_iii);
let sig1 = builder.addType(makeSig([kWasmF64, kWasmF64, kWasmI32,
kWasmI32, kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32, kWasmF32,
kWasmI32, kWasmF32, kWasmI32, kWasmF64, kWasmI32], [kWasmI32]));
let main = builder.addFunction("main", sig0)
.addBody([
kExprI64Const, 0,
kExprF64UConvertI64,
kExprF64Const, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x00, 0x00,
kExprF64Const, 0x30, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64Mul,
kExprI32Const, 0,
kExprF64Const, 0x30, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprF64StoreMem, 0x00, 0xb0, 0xe0, 0xc0, 0x81, 0x03,
kExprI32Const, 0,
kExprI32Const, 0,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF32Const, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprF64Const, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
kExprI32Const, 0,
kExprI32Const, 2,
kExprReturnCallIndirect, sig1, kTableZero]).exportFunc();
builder.addFunction("f", sig1).addBody([kExprI32Const, 0]);
builder.addTable(kWasmAnyFunc, 4, 4);
builder.addMemory(16, 32, false, true);
let module = new WebAssembly.Module(builder.toBuffer());
let instance = new WebAssembly.Instance(module);
})();

0 comments on commit 70f622b

Please sign in to comment.