Details and patch coming.
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>.
<rdar://problem/93270571>