WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
206436
Air O0 should have better stack allocation
https://bugs.webkit.org/show_bug.cgi?id=206436
Summary
Air O0 should have better stack allocation
Saam Barati
Reported
2020-01-17 13:01:12 PST
We already have liveness, so using it for a simple stack allocator should be easy
Attachments
patch
(11.85 KB, patch)
2020-01-17 13:42 PST
,
Saam Barati
no flags
Details
Formatted Diff
Diff
patch
(11.83 KB, patch)
2020-01-17 14:31 PST
,
Saam Barati
no flags
Details
Formatted Diff
Diff
patch
(11.85 KB, patch)
2020-01-17 14:33 PST
,
Saam Barati
tzagallo
: review+
Details
Formatted Diff
Diff
patch for landing
(11.98 KB, patch)
2020-01-17 17:30 PST
,
Saam Barati
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Saam Barati
Comment 1
2020-01-17 13:01:46 PST
This fixes our Wasm stack overflow issues
Saam Barati
Comment 2
2020-01-17 13:01:59 PST
***
Bug 201026
has been marked as a duplicate of this bug. ***
Saam Barati
Comment 3
2020-01-17 13:02:08 PST
***
Bug 200918
has been marked as a duplicate of this bug. ***
Saam Barati
Comment 4
2020-01-17 13:42:57 PST
Created
attachment 388084
[details]
patch I just need to adjust my stack limit test since we change stack size in our tests
Saam Barati
Comment 5
2020-01-17 14:31:57 PST
Created
attachment 388090
[details]
patch
Saam Barati
Comment 6
2020-01-17 14:33:45 PST
Created
attachment 388091
[details]
patch
Saam Barati
Comment 7
2020-01-17 14:35:06 PST
Comment on
attachment 388091
[details]
patch View in context:
https://bugs.webkit.org/attachment.cgi?id=388091&action=review
> Source/JavaScriptCore/ChangeLog:9 > + code creating huge stack frames and causing simple Wasm code to stack
I'll fix "and causing" => "was causing"
Saam Barati
Comment 8
2020-01-17 14:48:29 PST
***
Bug 206419
has been marked as a duplicate of this bug. ***
Tadeu Zagallo
Comment 9
2020-01-17 15:06:10 PST
Comment on
attachment 388091
[details]
patch r=me
Radar WebKit Bug Importer
Comment 10
2020-01-17 17:29:52 PST
<
rdar://problem/58702105
>
Saam Barati
Comment 11
2020-01-17 17:30:41 PST
Created
attachment 388122
[details]
patch for landing
WebKit Commit Bot
Comment 12
2020-01-17 19:24:02 PST
The commit-queue encountered the following flaky tests while processing
attachment 388122
[details]
: editing/spelling/spellcheck-async-remove-frame.html
bug 158401
(authors:
morrita@google.com
,
rniwa@webkit.org
, and
tony@chromium.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 13
2020-01-17 19:24:35 PST
Comment on
attachment 388122
[details]
patch for landing Clearing flags on attachment: 388122 Committed
r254788
: <
https://trac.webkit.org/changeset/254788
>
WebKit Commit Bot
Comment 14
2020-01-17 19:24:37 PST
All reviewed patches have been landed. Closing bug.
Aakash Jain
Comment 15
2020-01-20 09:12:55 PST
> Committed
r254788
: <
https://trac.webkit.org/changeset/254788
>
This seems to have broken 6 JSC tests, as also indicated by EWS. JSC stress test failures: - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit-validate-phases - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit-validate-phases
Saam Barati
Comment 16
2020-01-20 10:37:21 PST
(In reply to Aakash Jain from
comment #15
)
> > Committed
r254788
: <
https://trac.webkit.org/changeset/254788
> > This seems to have broken 6 JSC tests, as also indicated by EWS. > > JSC stress test failures: > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > - > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > validate-phases > - > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > validate-phases
This seems super unlikely. These aren't wasm tests, right?
Saam Barati
Comment 17
2020-01-20 10:41:31 PST
(In reply to Saam Barati from
comment #16
)
> (In reply to Aakash Jain from
comment #15
) > > > Committed
r254788
: <
https://trac.webkit.org/changeset/254788
> > > This seems to have broken 6 JSC tests, as also indicated by EWS. > > > > JSC stress test failures: > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > > - > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > > validate-phases > > - > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > > validate-phases > > This seems super unlikely. These aren't wasm tests, right?
Also, a lot of these tests can't even hit this code path. Are you sure it wasn't a patch before mine?
Saam Barati
Comment 18
2020-01-20 10:43:43 PST
(In reply to Saam Barati from
comment #17
)
> (In reply to Saam Barati from
comment #16
) > > (In reply to Aakash Jain from
comment #15
) > > > > Committed
r254788
: <
https://trac.webkit.org/changeset/254788
> > > > This seems to have broken 6 JSC tests, as also indicated by EWS. > > > > > > JSC stress test failures: > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl > > > - mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint > > > - > > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit- > > > validate-phases > > > - > > > mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit- > > > validate-phases > > > > This seems super unlikely. These aren't wasm tests, right? > > Also, a lot of these tests can't even hit this code path. Are you sure it > wasn't a patch before mine?
Ugh, maybe it was my use of "requireOptions". I wonder if we forgot to clear that value and then we end up running this test with a super small stack height.
Aakash Jain
Comment 19
2020-01-20 10:47:08 PST
(In reply to Saam Barati from
comment #17
)
> Also, a lot of these tests can't even hit this code path. Are you sure it wasn't a patch before mine?
r254787
passed in
https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/145
254788 failed in
https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/146
Aakash Jain
Comment 20
2020-01-20 10:48:17 PST
From
https://build.webkit.org/builders/Apple-Catalina-Release-JSC-Tests/builds/146/steps/jscore-test/logs/stdio
mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-no-ftl: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-llint: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-baseline: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-dfg-eager-no-cjit-validate-phases: Exception: RangeError: Maximum call stack size exceeded. mozilla-tests.yaml/js1_5/Regress/regress-191633.js.mozilla-ftl-eager-no-cjit-validate-phases: Exception: RangeError: Maximum call stack size exceeded.
Alexey Proskuryakov
Comment 21
2020-01-20 12:52:17 PST
The regression is tracked in
bug 206477
.
Joey Krug
Comment 22
2020-01-29 18:55:25 PST
Noticed this is still happening, getting RangeError: Maximum call stack size exceeded. upon instantiation of WASM code. If I refresh, it tends to fix it, but only if I refresh things. On older safari (13.0) it doesn't work at all, so this is an improvement, but it doesn't seem to be totally fixed
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug