Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[JSC] Make megamorphic IC stateless #28714

Merged
merged 1 commit into from
May 18, 2024
Merged

[JSC] Make megamorphic IC stateless #28714

merged 1 commit into from
May 18, 2024

Conversation

Constellation
Copy link
Member

@Constellation Constellation commented May 17, 2024 •
edited by webkit-early-warning-system

6e5a36b

[JSC] Make megamorphic IC stateless
https://bugs.webkit.org/show_bug.cgi?id=274316
rdar://128277946

Reviewed by Keith Miller.

We store expected UniquedStringImpl into InlineCacheHandler (must not keep it as Ref! lifetime is managed by StructureStubInfo) in megamorphic IC
when Handler IC is enabled. And we retrieve it from GPRInfo::handlerGPR. So we can share one code for all of megamorphic IC, thus it becomes stateless.

* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::isStateless):
(JSC::InlineCacheHandler::InlineCacheHandler):
(JSC::InlineCacheCompiler::generateWithGuard):
(JSC::InlineCacheCompiler::regenerate):
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.h:

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

a231c2e

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

@Constellation Constellation requested a review from a team as a code owner May 17, 2024 16:29
@Constellation Constellation self-assigned this May 17, 2024
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label May 17, 2024
@Constellation Constellation force-pushed the eng/JSC-Make-megamorphic-IC-stateless branch from 2def74d to 3c0e06e Compare May 17, 2024 16:29
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 2def74d)

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

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 3c0e06e)

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

kmiller68
kmiller68 approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@kmiller68 kmiller68 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me.

Source/JavaScriptCore/bytecode/InlineCacheCompiler.h

static Ref<InlineCacheHandler> createSlowPath(VM&, AccessType);

CodePtr<JITStubRoutinePtrTag> m_callTarget;
CodePtr<JITStubRoutinePtrTag> m_jumpTarget;
UniquedStringImpl* m_uid { nullptr };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we could use CheckedPtr<UniquedStringImpl> here but I guess StringImpl isn't CanMakeCheckedPtr

@Constellation Constellation force-pushed the eng/JSC-Make-megamorphic-IC-stateless branch from 3c0e06e to a231c2e Compare May 18, 2024 00:23
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash a231c2e)

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

@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 18, 2024
@Constellation
[JSC] Make megamorphic IC stateless
6e5a36b 
https://bugs.webkit.org/show_bug.cgi?id=274316
rdar://128277946

Reviewed by Keith Miller.

We store expected UniquedStringImpl into InlineCacheHandler (must not keep it as Ref! lifetime is managed by StructureStubInfo) in megamorphic IC
when Handler IC is enabled. And we retrieve it from GPRInfo::handlerGPR. So we can share one code for all of megamorphic IC, thus it becomes stateless.

* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::isStateless):
(JSC::InlineCacheHandler::InlineCacheHandler):
(JSC::InlineCacheCompiler::generateWithGuard):
(JSC::InlineCacheCompiler::regenerate):
* Source/JavaScriptCore/bytecode/InlineCacheCompiler.h:

Canonical link: https://commits.webkit.org/278946@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/JSC-Make-megamorphic-IC-stateless branch from a231c2e to 6e5a36b Compare May 18, 2024 04:25
@webkit-commit-queue
Copy link
Collaborator

Committed 278946@main (6e5a36b): https://commits.webkit.org/278946@main

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

@webkit-commit-queue webkit-commit-queue merged commit 6e5a36b into WebKit:main May 18, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label May 18, 2024
@Constellation Constellation deleted the eng/JSC-Make-megamorphic-IC-stateless branch May 19, 2024 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kmiller68 kmiller68 kmiller68 approved these changes

Assignees

@Constellation Constellation

Labels
JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.
Projects
None yet
Milestone
No milestone
4 participants
@Constellation @webkit-early-warning-system @webkit-commit-queue @kmiller68