[JSC] Use handler IC for single stateless AccessCase by Constellation · Pull Request #25468 · WebKit/WebKit

Constellation · 2024-03-05T03:09:29Z

`540d08d`

[JSC] Use handler IC for single stateless AccessCase
https://bugs.webkit.org/show_bug.cgi?id=270497
rdar://124047629

Reviewed by Keith Miller.

This patch enables Handler IC only for very specific case: single stateless AccessCase.
For example, ArrayLength, IndexedContiguousLoad etc. does not care about Structure. They
only care about the input's type. So the underlying code can be reused in different places completely.
And if AccessCase is only one, the generated code can be reused in various places. And surprisingly
this is relatively frequently happening.

1. This patch categorizes stateless AccessCases. They do not require Structure etc. state of the heap.
2. We clean up InlineCacheCompiler implementation about accessing to StructureStubInfo* to figure out
what is the values changing the generated code from StructureStubInfo.
3. We clean up InlineCacheCompiler's information collection code from vector of AccessCase so that we
can easily see what information is collected.
4. We extend SharedJITStubSet to store stateless stubs. Previously it was only storing megamorphic stubs
since they are stateless. But now it gets extended to accept all stateless stubs.

* Source/JavaScriptCore/bytecode/AccessCase.h:
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::isStateless):
(JSC::InlineCacheCompiler::regenerate):
(WTF::printInternal):
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.h:
* Source/JavaScriptCore/bytecode/StructureStubInfo.cpp:
(JSC::SharedJITStubSet::getStatelessStub const):
(JSC::SharedJITStubSet::setStatelessStub):
(JSC::SharedJITStubSet::getMegamorphic const): Deleted.
(JSC::SharedJITStubSet::setMegamorphic): Deleted.
* Source/JavaScriptCore/bytecode/StructureStubInfo.h:

Canonical link: https://commits.webkit.org/275721@main

50eaf44

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	~~🛠 ios~~	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	~~🛠 ios-sim~~	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	~~🛠 tv~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	~~🛠 tv-sim~~		~~🧪 api-gtk~~
	~~🛠 watch~~		✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge	~~🛠 watch-sim~~		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-03-05T03:09:39Z

EWS run on previous version of this PR (hash 0a572e0)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🧪 api-wpe
	⏳ 🧪 ios-wk2-wpt	~~🧪 mac-wk1~~	✅ 🛠 gtk
~~🛠 🧪 jsc~~	⏳ 🧪 api-ios	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	⏳ 🧪 mac-AS-debug-wk2	⏳ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
	~~🛠 watch~~		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-03-05T05:51:04Z

EWS run on previous version of this PR (hash 4d7fc9a)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
	~~🛠 ios-sim~~	~~🛠 mac-AS-debug~~	~~🧪 wpe-wk2~~
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 gtk~~
~~🛠 🧪 jsc~~	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
~~🛠 🧪 jsc-arm64~~	~~🛠 tv~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 api-gtk~~
	~~🛠 tv-sim~~	~~🧪 mac-wk2-stress~~	~~🛠 jsc-armv7~~
	~~🛠 watch~~		~~🧪 jsc-armv7-tests~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-03-05T05:51:05Z

EWS run on previous version of this PR (hash bd15a53)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
~~🧪 style~~	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 wincairo~~
	~~🛠 ios-sim~~	~~🛠 mac-AS-debug~~	~~🧪 wpe-wk2~~
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 gtk~~
~~🛠 🧪 jsc~~	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🧪 gtk-wk2~~
~~🛠 🧪 jsc-arm64~~	~~🛠 tv~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 api-gtk~~
	~~🛠 tv-sim~~	~~🧪 mac-wk2-stress~~	~~🛠 jsc-armv7~~
	~~🛠 watch~~		~~🧪 jsc-armv7-tests~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-03-05T05:52:01Z

EWS run on previous version of this PR (hash 0288636)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-03-05T21:12:34Z

EWS run on previous version of this PR (hash 1ed1433)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	~~🛠 mac-AS-debug~~	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🧪 api-mac	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	✅ 🛠 wpe-skia
~~🛠 🧪 jsc~~	⏳ 🧪 api-ios	⏳ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	✅ 🛠 tv-sim		⏳ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		⏳ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-03-05T21:34:18Z

EWS run on previous version of this PR (hash 4dcd882)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	~~🛠 ios-sim~~	~~🛠 mac-AS-debug~~	⏳ 🧪 wpe-wk2
~~🧪 webkitperl~~	~~🧪 ios-wk2~~	⏳ 🧪 api-mac	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	~~🧪 mac-wk1~~	✅ 🛠 wpe-skia
~~🛠 🧪 jsc~~	~~🧪 api-ios~~	⏳ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~	⏳ 🧪 gtk-wk2
	✅ 🛠 tv-sim		⏳ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		⏳ 🧪 jsc-armv7-tests

webkit-early-warning-system · 2024-03-05T22:04:50Z

EWS run on previous version of this PR (hash c99e279)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
~~🛠 🧪 jsc~~	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim		✅ 🧪 api-gtk
	✅ 🛠 watch		✅ 🛠 jsc-armv7
	✅ 🛠 watch-sim		✅ 🧪 jsc-armv7-tests

kmiller68

r=me with comments

kmiller68 · 2024-03-05T21:50:32Z

Source/JavaScriptCore/bytecode/StructureStubInfo.h

It'd be nice if these were enums for readability.

It was actually hard to see it in enum for all of these bools in various places. I feel like using bool is better here since this is not randomly used key. It is only created from StructureStubInfo, for InlineCacheCompiler.
If this is used and created in various places, I think enum would be good, or using struct here would be nice. But given that it is only used in one place, I think this is simpler and better (plus, better for extensibility).

kmiller68 · 2024-03-05T21:51:55Z

Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp

What's up with this removal and the follow on stubInfo. -> m_stubInfo-> changes? Seems like unnecessary code churn. If the concern is that we can't tell who's accessing stateful StructureStubInfo members we should guard access behind ASSERTs in getters.

Using m_stubInfo actually found dependent use of stubInfo information in code generation, so I think it is definitely better than using stubInfo. in random places (consistent use of m_stubInfo). Right now, asserting is very hard since it depends on DataIC / RepatchingIC, and making code super complicated.

kmiller68 · 2024-03-05T21:55:46Z

Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp

"contains the cases" => "contains cases"

webkit-early-warning-system · 2024-03-06T00:18:23Z

EWS run on current version of this PR (hash 50eaf44)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	~~🛠 ios~~	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	~~🛠 ios-sim~~	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
✅ 🛠 🧪 jsc	~~🧪 api-ios~~	✅ 🧪 mac-wk2	✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64	~~🛠 tv~~	~~🧪 mac-AS-debug-wk2~~	~~🧪 gtk-wk2~~
	~~🛠 tv-sim~~		~~🧪 api-gtk~~
	~~🛠 watch~~		✅ 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge	~~🛠 watch-sim~~		✅ 🧪 jsc-armv7-tests

https://bugs.webkit.org/show_bug.cgi?id=270497 rdar://124047629 Reviewed by Keith Miller. This patch enables Handler IC only for very specific case: single stateless AccessCase. For example, ArrayLength, IndexedContiguousLoad etc. does not care about Structure. They only care about the input's type. So the underlying code can be reused in different places completely. And if AccessCase is only one, the generated code can be reused in various places. And surprisingly this is relatively frequently happening. 1. This patch categorizes stateless AccessCases. They do not require Structure etc. state of the heap. 2. We clean up InlineCacheCompiler implementation about accessing to StructureStubInfo* to figure out what is the values changing the generated code from StructureStubInfo. 3. We clean up InlineCacheCompiler's information collection code from vector of AccessCase so that we can easily see what information is collected. 4. We extend SharedJITStubSet to store stateless stubs. Previously it was only storing megamorphic stubs since they are stateless. But now it gets extended to accept all stateless stubs. * Source/JavaScriptCore/bytecode/AccessCase.h: * Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp: (JSC::isStateless): (JSC::InlineCacheCompiler::regenerate): (WTF::printInternal): * Source/JavaScriptCore/bytecode/InlineCacheCompiler.h: * Source/JavaScriptCore/bytecode/StructureStubInfo.cpp: (JSC::SharedJITStubSet::getStatelessStub const): (JSC::SharedJITStubSet::setStatelessStub): (JSC::SharedJITStubSet::getMegamorphic const): Deleted. (JSC::SharedJITStubSet::setMegamorphic): Deleted. * Source/JavaScriptCore/bytecode/StructureStubInfo.h: Canonical link: https://commits.webkit.org/275721@main

webkit-commit-queue · 2024-03-06T01:32:32Z

Committed 275721@main (540d08d): https://commits.webkit.org/275721@main

Reviewed commits have been landed. Closing PR #25468 and removing active labels.

Constellation self-assigned this Mar 5, 2024

Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Mar 5, 2024

Constellation force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch 3 times, most recently from bd15a53 to 0288636 Compare March 5, 2024 05:18

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 5, 2024

Constellation removed the merging-blocked Applied to prevent a change from being merged label Mar 5, 2024

Constellation force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch from 0288636 to 1ed1433 Compare March 5, 2024 21:11

Constellation force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch from 1ed1433 to 4dcd882 Compare March 5, 2024 21:34

Constellation marked this pull request as ready for review March 5, 2024 21:34

Constellation requested a review from a team as a code owner March 5, 2024 21:34

Constellation force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch from 4dcd882 to c99e279 Compare March 5, 2024 22:00

kmiller68 approved these changes Mar 5, 2024

View reviewed changes

Constellation force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch from c99e279 to 50eaf44 Compare March 6, 2024 00:14

Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 6, 2024

webkit-commit-queue force-pushed the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch from 50eaf44 to 540d08d Compare March 6, 2024 01:32

webkit-commit-queue merged commit 540d08d into WebKit:main Mar 6, 2024

webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 6, 2024

Constellation deleted the eng/JSC-Use-handler-IC-for-single-stateless-AccessCase branch March 6, 2024 01:53

Conversation

Constellation commented Mar 5, 2024 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

540d08d

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kmiller68 left a comment

Choose a reason for hiding this comment

Uh oh!

kmiller68 Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

Constellation Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

kmiller68 Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

Constellation Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

kmiller68 Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

Constellation Mar 6, 2024

Choose a reason for hiding this comment

Uh oh!

webkit-early-warning-system commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-commit-queue commented Mar 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Constellation commented Mar 5, 2024 •

edited by webkit-early-warning-system

Loading

`540d08d`

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 5, 2024 •

edited

Loading

webkit-early-warning-system commented Mar 6, 2024 •

edited

Loading