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

Add some JIT comments to make debugging easier #13842

Merged
merged 1 commit into from
May 17, 2023
Merged

Add some JIT comments to make debugging easier #13842

merged 1 commit into from
May 17, 2023

Conversation

justinmichaud
Copy link
Contributor

@justinmichaud justinmichaud commented May 13, 2023 •
edited by webkit-early-warning-system

d96b776

Add some JIT comments to make debugging easier
https://bugs.webkit.org/show_bug.cgi?id=256745
rdar://109288342

Reviewed by Yusuke Suzuki.

Add some JIT comments to make debugging easier

* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::InlineCacheCompiler::generateWithGuard):
(JSC::InlineCacheCompiler::regenerate):
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByIdMegamorphic):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValMegamorphic):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValWithThis):
(JSC::FTL::DFG::LowerDFGToB3::getPrivateName):
(JSC::FTL::DFG::LowerDFGToB3::compilePrivateBrandAccess):
(JSC::FTL::DFG::LowerDFGToB3::compilePutPrivateName):
(JSC::FTL::DFG::LowerDFGToB3::cachedPutById):
(JSC::FTL::DFG::LowerDFGToB3::emitGetTypedArrayByteOffsetExceptSettingResult):
(JSC::FTL::DFG::LowerDFGToB3::typedArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValImpl):
(JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileDelBy):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* Source/JavaScriptCore/ftl/FTLOutput.cpp:
(JSC::FTL::Output::probeDebugPrint):
* Source/JavaScriptCore/ftl/FTLOutput.h:

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

bf61026

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

@justinmichaud justinmichaud requested a review from a team as a code owner May 13, 2023 01:11
@justinmichaud justinmichaud self-assigned this May 13, 2023
@justinmichaud justinmichaud added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label May 13, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 6c4d378)

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 13, 2023
@justinmichaud justinmichaud force-pushed the eng/Add-some-JIT-comments-to-make-debugging-easier branch from 6c4d378 to 4038d1a Compare May 15, 2023 16:54
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 4038d1a)

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

@hyjorc1
Copy link
Contributor

hyjorc1 commented May 15, 2023

LGTM

@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label May 15, 2023
Constellation
Constellation approved these changes May 15, 2023
View reviewed changes
Copy link
Member

@Constellation Constellation left a comment

Choose a reason for hiding this comment

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

rs=me

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4044,6 +4045,7 @@ class LowerDFGToB3 {
CodeOrigin nodeSemanticOrigin = m_node->origin.semantic;
CacheableIdentifier identifier = m_node->cacheableIdentifier();
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compileGetByIdMegamorphic");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -3853,6 +3853,7 @@ class LowerDFGToB3 {
patchpoint->appendSomeRegister(val);
patchpoint->setGenerator(
[=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compileAssertNotEmpty");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4100,6 +4102,7 @@ class LowerDFGToB3 {
State* state = &m_ftlState;
CodeOrigin nodeSemanticOrigin = m_node->origin.semantic;
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compileGetByValMegamorphic");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4280,6 +4283,7 @@ class LowerDFGToB3 {
State* state = &m_ftlState;
CodeOrigin nodeSemanticOrigin = node->origin.semantic;
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compileGetByValWithThis");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4437,6 +4441,7 @@ class LowerDFGToB3 {
bool baseIsCell = abstractValue(node->child1()).isType(SpecCell);
CodeOrigin nodeSemanticOrigin = node->origin.semantic;
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compileGetPrivateName");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4585,6 +4590,7 @@ class LowerDFGToB3 {
bool baseIsCell = abstractValue(m_node->child1()).isType(SpecCell);
CodeOrigin nodeSemanticOrigin = node->origin.semantic;
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compilePrivateBrandAccess");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp Outdated
@@ -4747,6 +4753,7 @@ class LowerDFGToB3 {
PrivateFieldPutKind privateFieldPutKind = m_node->privateFieldPutKind();
auto operation = privateFieldPutKind.isDefine() ? operationPutByValDefinePrivateFieldOptimize : operationPutByValSetPrivateFieldOptimize;
patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
JIT_COMMENT(jit, "compilePutPrivateName");
Copy link
Member

Choose a reason for hiding this comment

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

compile is not necessary.

ddegazio
ddegazio reviewed May 15, 2023
View reviewed changes
Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp
@@ -374,6 +374,7 @@ void InlineCacheCompiler::generateWithGuard(AccessCase& accessCase, CCallHelpers

JSGlobalObject* globalObject = m_globalObject;
CCallHelpers& jit = *m_jit;
JIT_COMMENT(jit, "Begin generateWithGuard");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there supposed to be a matching "End generateWithGuard" comment here? If not, maybe that'd be a useful addition.

@justinmichaud justinmichaud force-pushed the eng/Add-some-JIT-comments-to-make-debugging-easier branch from 4038d1a to bf61026 Compare May 17, 2023 20:37
@justinmichaud justinmichaud added the merge-queue Applied to send a pull request to merge-queue label May 17, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

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

@justinmichaud
Add some JIT comments to make debugging easier
d96b776 
https://bugs.webkit.org/show_bug.cgi?id=256745
rdar://109288342

Reviewed by Yusuke Suzuki.

Add some JIT comments to make debugging easier

* Source/JavaScriptCore/bytecode/InlineCacheCompiler.cpp:
(JSC::InlineCacheCompiler::generateWithGuard):
(JSC::InlineCacheCompiler::regenerate):
* Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByIdMegamorphic):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValMegamorphic):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValWithThis):
(JSC::FTL::DFG::LowerDFGToB3::getPrivateName):
(JSC::FTL::DFG::LowerDFGToB3::compilePrivateBrandAccess):
(JSC::FTL::DFG::LowerDFGToB3::compilePutPrivateName):
(JSC::FTL::DFG::LowerDFGToB3::cachedPutById):
(JSC::FTL::DFG::LowerDFGToB3::emitGetTypedArrayByteOffsetExceptSettingResult):
(JSC::FTL::DFG::LowerDFGToB3::typedArrayLength):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByValImpl):
(JSC::FTL::DFG::LowerDFGToB3::compilePutByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileDelBy):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
* Source/JavaScriptCore/ftl/FTLOutput.cpp:
(JSC::FTL::Output::probeDebugPrint):
* Source/JavaScriptCore/ftl/FTLOutput.h:

Canonical link: https://commits.webkit.org/264180@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Add-some-JIT-comments-to-make-debugging-easier branch from bf61026 to d96b776 Compare May 17, 2023 21:59
@webkit-commit-queue
Copy link
Collaborator

Committed 264180@main (d96b776): https://commits.webkit.org/264180@main

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

@webkit-commit-queue webkit-commit-queue merged commit d96b776 into WebKit:main May 17, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 17, 2023
@justinmichaud justinmichaud deleted the eng/Add-some-JIT-comments-to-make-debugging-easier branch March 20, 2024 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddegazio ddegazio ddegazio left review comments

@Constellation Constellation Constellation approved these changes

Assignees

@justinmichaud justinmichaud

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