Summary: | Enhance the ARM64Disassembler to print pc indices and better branch target labels. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Mark Lam
2022-05-12 23:16:59 PDT
Created attachment 459277 [details]
proposed patch.
Comment on attachment 459277 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=459277&action=review > Source/JavaScriptCore/ChangeLog:47 > + Introduced a FINALIZE_AND_RETURN_THUNK macro that will be used instead of > + "return FINALIZE_CODE". By doing so, thunk labels will automatically be The first sentence here should read: Introduced a FINALIZE_AND_RETURN_THUNK macro that will be used instead of "return FINALIZE_CODE" in thunk generators." Will fix before landing. Created attachment 459279 [details]
proposed patch.
Comment on attachment 459279 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=459279&action=review Nice. r=me > Source/JavaScriptCore/ChangeLog:33 > + 2. Relative branches now show the branch target as a pc offset in addition to the > + pc address e.g. maybe "pc offset" => "offset from the start of the function"? > Source/JavaScriptCore/ChangeLog:44 > + <828> 0x10e12033c: bl 0x10e0f0a00 -> <thunk: get_from_scope thunk> > + <1476> 0x10e120dc4: cbnz x16, 0x10e104100 -> <thunk: handleExceptionWithCallFrameRollback> > + <2368> 0x10e121140: b 0x10e10c000 -> <thunk: DFG OSR exit generation thunk> Nice! > Source/JavaScriptCore/ChangeLog:57 > + <168> 0x10e1002e8: b 0x10e120b60 -> <external: JIT PC> I think based on this algorithm, you mean external just in the sense that it's not owned by this exact compilation. It still might be "internal" to this particular CodeBlock. I wonder if this may confuse some people. But maybe it's fine. I'd be tempted to just label all three of these things in this section without "external: " prefixing them. so just <LLInt PC>, <JIT PC>, <unknown> > Source/JavaScriptCore/llint/LLIntThunks.cpp:112 > + FINALIZE_AND_RETURN_THUNK(patchBuffer, tag, "LLInt %s jump to prologue thunk", thunkKind); I think this is more characters than just "return FINALIZE_THUNK", and doing the return manually is less magic Comment on attachment 459279 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=459279&action=review Thanks for the review. >> Source/JavaScriptCore/ChangeLog:33 >> + pc address e.g. > > maybe "pc offset" => "offset from the start of the function"? Fixed. >> Source/JavaScriptCore/ChangeLog:57 >> + <168> 0x10e1002e8: b 0x10e120b60 -> <external: JIT PC> > > I think based on this algorithm, you mean external just in the sense that it's not owned by this exact compilation. It still might be "internal" to this particular CodeBlock. I wonder if this may confuse some people. But maybe it's fine. > > I'd be tempted to just label all three of these things in this section without "external: " prefixing them. > > so just <LLInt PC>, <JIT PC>, <unknown> SGTM. Changed. >> Source/JavaScriptCore/llint/LLIntThunks.cpp:112 >> + FINALIZE_AND_RETURN_THUNK(patchBuffer, tag, "LLInt %s jump to prologue thunk", thunkKind); > > I think this is more characters than just "return FINALIZE_THUNK", and doing the return manually is less magic The only reason I did this was because I needed to execute another statement before evaluating FINALIZE_CODE to produce the result that would be returned. However, it looks like I can use a comma separated expression list to achieve this. So, I'll go ahead and change all these to "return FINALIZE_THUNK". Created attachment 459323 [details]
patch for landing.
Landed in r294180: <http://trac.webkit.org/r294180>. |