Bug 227798

Summary: display-profiler-output should be able to print disassembly for the FTL
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, mark.lam, msaboff, rmorisset, saam, tzagallo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch saam: review+

Description Keith Miller 2021-07-08 09:08:59 PDT
display-profiler-output should be able to print disassembly for the FTL
Comment 1 Keith Miller 2021-07-08 09:12:30 PDT
Created attachment 433135 [details]
Patch
Comment 2 Robin Morisset 2021-07-08 10:02:11 PDT
Comment on attachment 433135 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433135&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Right now running JSC with the bytecode profiler will will not print

typo: will will

> Source/JavaScriptCore/ChangeLog:11
> +        before the DFG graph despite being printed after before and being added

typo: after before

> Source/JavaScriptCore/ChangeLog:12
> +        to the Profiler::Compilation after the graph. Lastly, the disassmebly

typo: disassmebly

> Source/JavaScriptCore/ChangeLog:13
> +        is the same text as Options::dumpDisassebly so it doesn't include execution

typo: Disassebly
Comment 3 Keith Miller 2021-07-08 10:40:45 PDT
Created attachment 433140 [details]
Patch
Comment 4 Keith Miller 2021-07-08 10:45:07 PDT
Created attachment 433141 [details]
Patch
Comment 5 Keith Miller 2021-07-08 10:49:41 PDT
Created attachment 433142 [details]
Patch
Comment 6 Saam Barati 2021-07-08 11:06:36 PDT
Comment on attachment 433142 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433142&action=review

> Source/JavaScriptCore/ChangeLog:26
> +        94908                 979/4095/94908/0             Air               Move -824(%rax), %rax, b@402
> +        94908                 979/4095/94908/0             asm                   0x3bf95fa085df: mov -0x338(%rax), %rax
> +        94908                 979/4095/94908/0             b3            Void b@403 = Patchpoint($4396662888(b@90):ColdAny, generator = 0x1066c02e0, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx], resultConstraints = WarmAny, WritesLocalState|Reads:Top, D@65)
> +        94908                 979/4095/94908/0             Air               Patch &Patchpoint0, $0x1060fc068, b@403

It's very deceptive to show DFG node execution counts for B3/Air code. I'd recommend just not showing that info for B3/Air, but still showing it for the parent DFG node.
Comment 7 Saam Barati 2021-07-08 11:07:42 PDT
(In reply to Saam Barati from comment #6)
> Comment on attachment 433142 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=433142&action=review
> 
> > Source/JavaScriptCore/ChangeLog:26
> > +        94908                 979/4095/94908/0             Air               Move -824(%rax), %rax, b@402
> > +        94908                 979/4095/94908/0             asm                   0x3bf95fa085df: mov -0x338(%rax), %rax
> > +        94908                 979/4095/94908/0             b3            Void b@403 = Patchpoint($4396662888(b@90):ColdAny, generator = 0x1066c02e0, earlyClobbered = [], lateClobbered = [], usedRegisters = [%rax, %rbx], resultConstraints = WarmAny, WritesLocalState|Reads:Top, D@65)
> > +        94908                 979/4095/94908/0             Air               Patch &Patchpoint0, $0x1060fc068, b@403
> 
> It's very deceptive to show DFG node execution counts for B3/Air code. I'd
> recommend just not showing that info for B3/Air, but still showing it for
> the parent DFG node.

My reasoning is this:

Many DFG nodes lower to a lot of control flow in B3. Better to not incorrectly say all that control flow executes the same number of times.
Comment 8 Keith Miller 2021-07-08 11:30:43 PDT
Created attachment 433147 [details]
Patch
Comment 9 Keith Miller 2021-07-08 11:54:59 PDT
Landed in r279740.