Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(137)

Issue 2763593002: [wasm][arm] Add an additional stack check for functions with big frames. (Closed)

Created:
3 years, 9 months ago by ahaas
Modified:
3 years, 9 months ago
Reviewers:
v8-arm-ports, Michael Starzinger, Rodolph Perfetta
CC:
v8-mips-ports_googlegroups.com, v8-ppc-ports_googlegroups.com, v8-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

[wasm][arm] Add an additional stack check for functions with big frames. Stack overflow checks are typically implemented as part of the TurboFan graph of a function. This means that the stack check code is executed after frame construction. When a frame is too big, though, there may not be enough space on the stack anymore to throw the stack overflow exception after frame construction. With this CL we do an additional stack check before frame construction for functions with big frames. As discussed offline with mstarzinger, I do this change currently only for WebAssembly. This CL contains only the changes for arm. I will do the other platforms in separate CLs. R=mstarzinger@chromium.org, v8-arm-ports@googlegroups.com Review-Url: https://codereview.chromium.org/2763593002 Cr-Commit-Position: refs/heads/master@{#44065} Committed: https://chromium.googlesource.com/v8/v8/+/9f01d5c1e0c634a0d6e40a514ac7e49382b5abe3

Patch Set 1 #

Total comments: 4

Patch Set 2 : Comments addressed #

Total comments: 3

Patch Set 3 : Load the actual stack limit. #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -0 lines) Patch
M src/compiler/arm/code-generator-arm.cc View 1 2 1 chunk +41 lines, -0 lines 3 comments Download
M src/runtime/runtime.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime-wasm.cc View 1 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (17 generated)
ahaas
3 years, 9 months ago (2017-03-20 13:50:40 UTC) #1
Michael Starzinger
LGTM. https://codereview.chromium.org/2763593002/diff/1/src/runtime/runtime-wasm.cc File src/runtime/runtime-wasm.cc (right): https://codereview.chromium.org/2763593002/diff/1/src/runtime/runtime-wasm.cc#newcode143 src/runtime/runtime-wasm.cc:143: SealHandleScope shs(isolate); nit: The {Isolate::StackOverflow} method creates its ...
3 years, 9 months ago (2017-03-20 14:54:52 UTC) #6
ahaas
https://codereview.chromium.org/2763593002/diff/1/src/runtime/runtime-wasm.cc File src/runtime/runtime-wasm.cc (right): https://codereview.chromium.org/2763593002/diff/1/src/runtime/runtime-wasm.cc#newcode143 src/runtime/runtime-wasm.cc:143: SealHandleScope shs(isolate); On 2017/03/20 at 14:54:51, Michael Starzinger wrote: ...
3 years, 9 months ago (2017-03-20 15:16:23 UTC) #9
Rodolph Perfetta
DBC https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc File src/compiler/arm/code-generator-arm.cc (right): https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc#newcode2417 src/compiler/arm/code-generator-arm.cc:2417: __ add(kScratchReg, kScratchReg, kScratchReg holds the address of ...
3 years, 9 months ago (2017-03-20 22:07:17 UTC) #13
Michael Starzinger
https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc File src/compiler/arm/code-generator-arm.cc (right): https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc#newcode2417 src/compiler/arm/code-generator-arm.cc:2417: __ add(kScratchReg, kScratchReg, On 2017/03/20 22:07:17, Rodolph Perfetta wrote: ...
3 years, 9 months ago (2017-03-21 09:13:51 UTC) #14
ahaas
https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc File src/compiler/arm/code-generator-arm.cc (right): https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc#newcode2417 src/compiler/arm/code-generator-arm.cc:2417: __ add(kScratchReg, kScratchReg, On 2017/03/21 at 09:13:50, Michael Starzinger ...
3 years, 9 months ago (2017-03-21 10:44:40 UTC) #19
ahaas
On 2017/03/21 at 10:44:40, ahaas wrote: > https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc > File src/compiler/arm/code-generator-arm.cc (right): > > https://codereview.chromium.org/2763593002/diff/20001/src/compiler/arm/code-generator-arm.cc#newcode2417 ...
3 years, 9 months ago (2017-03-23 07:52:05 UTC) #20
Rodolph Perfetta
https://codereview.chromium.org/2763593002/diff/40001/src/compiler/arm/code-generator-arm.cc File src/compiler/arm/code-generator-arm.cc (right): https://codereview.chromium.org/2763593002/diff/40001/src/compiler/arm/code-generator-arm.cc#newcode2446 src/compiler/arm/code-generator-arm.cc:2446: if (saves_fp != 0) { this code will decrement ...
3 years, 9 months ago (2017-03-23 13:02:20 UTC) #21
ahaas
https://codereview.chromium.org/2763593002/diff/40001/src/compiler/arm/code-generator-arm.cc File src/compiler/arm/code-generator-arm.cc (right): https://codereview.chromium.org/2763593002/diff/40001/src/compiler/arm/code-generator-arm.cc#newcode2446 src/compiler/arm/code-generator-arm.cc:2446: if (saves_fp != 0) { On 2017/03/23 at 13:02:20, ...
3 years, 9 months ago (2017-03-23 14:14:09 UTC) #22
Rodolph Perfetta
lgtm
3 years, 9 months ago (2017-03-23 14:20:54 UTC) #23
ahaas
On 2017/03/23 at 14:20:54, rodolph.perfetta wrote: > lgtm Thanks
3 years, 9 months ago (2017-03-23 14:25:09 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2763593002/40001
3 years, 9 months ago (2017-03-23 14:25:20 UTC) #27
commit-bot: I haz the power
3 years, 9 months ago (2017-03-23 15:41:00 UTC) #30
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/9f01d5c1e0c634a0d6e40a514ac7e49382b...

Powered by Google App Engine
This is Rietveld 408576698