Bug 200693 - Add phase, block, and node numbers to left margin of DFG graph dumps.
Summary: Add phase, block, and node numbers to left margin of DFG graph dumps.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-08-13 15:46 PDT by Mark Lam
Modified: 2019-08-13 16:33 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch. (22.75 KB, patch)
2019-08-13 15:54 PDT, Mark Lam
sbarati: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2019-08-13 15:46:17 PDT
When scrolling through the DFG graph dumps, it's easy to get lost as to which phase or block one is looking at, especially if the blocks are long.  This patch adds phase, block, and node numbers on the left margin of the dumps.  Here's a sample:

           53:     %Bd:Function                   = 0x1079fd960:[Function, {}, NonArray, Proto:0x1079d8000, Leaf]
           53:     %Bf:Function                   = 0x1079b0700:[Function, {name:100, prototype:101, length:102, stackTraceLimit:103}, NonArray, Proto:0x1079d8000, Leaf]
           53:     %Bj:Function                   = 0x1079fd5e0:[Function, {name:100, length:101, toString:102, apply:103, call:104, bind:105, Symbol.hasInstance:106, caller:107, arguments:108, constructor:109}, NonArray, Proto:0x1079c0000, Leaf]
           53:     %CV:JSGlobalLexicalEnvironment = 0x1079fd6c0:[JSGlobalLexicalEnvironment, {}, NonArray, Leaf]

           53: Phase liveness analysis changed the IR.

           54: Beginning DFG phase OSR availability analysis.
           54: Before OSR availability analysis:

           54: DFG for foo#DXMNag:[0x1079a4850->0x1079a4130->0x1079c7600, DFGFunctionCall, 204 (NeverInline)]:
           54:   Fixpoint state: FixpointConverged; Form: SSA; Unification state: GloballyUnified; Ref count state: ExactRefCount
           54:   Argument formats for entrypoint index: 0 : FlushedJSValue, FlushedCell, FlushedJSValue

         0 54: Block #0 (bc#0): (OSR target)
         0 54:   Execution count: 1.000000
         0 54:   Predecessors:
         0 54:   Successors:
         0 54:   Dominated by: #0
         0 54:   Dominates: #0
         0 54:   Dominance Frontier: 
         0 54:   Iterated Dominance Frontier: 
         0 54:   Backwards dominates by: #root #0
         0 54:   Backwards dominates: #0
         0 54:   Control equivalent to: #0
         0 54:   States: StructuresAreWatched
         0 54:   Live: 
         0 54:   Values: 
      0  0 54:   53:< 1:->	JSConstant(JS|UseAsOther, Other, Null, bc#0, ExitValid)
      1  0 54:   64:< 2:->	JSConstant(JS|UseAsOther, NonBoolInt32, Int32: 10, bc#0, ExitValid)
      2  0 54:    3:< 5:->	JSConstant(JS|PureInt, Other, Undefined, bc#0, ExitValid)
      3  0 54:   32:< 1:->	JSConstant(JS|UseAsOther, Bool, False, bc#0, ExitValid)
      4  0 54:   19:< 2:->	JSConstant(JS|UseAsOther, OtherObj, Weak:Object: 0x1079d4000 with butterfly 0x0 (Structure %CV:JSGlobalLexicalEnvironment), StructureID: 31423, bc#0, ExitValid)

The numbers in the left margin before the ':' are node number (index of node in the block), block number, and phase number respectively.  Now, we can scroll thru the dumps quickly and tell at a glance when we've scrolled passed the end of a phase, or block.  These sets of numbers can also serve as a positional marker that we can search for to return to a node in the dump after scrolling away.

Currently, these numbers are only added to the DFG part.  The FTL (from lowering to B3 onwards) does not have this feature yet.
Comment 1 Mark Lam 2019-08-13 15:54:15 PDT
Created attachment 376221 [details]
proposed patch.
Comment 2 Saam Barati 2019-08-13 16:02:41 PDT
Comment on attachment 376221 [details]
proposed patch.

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

> Source/JavaScriptCore/ChangeLog:46
> +        The numbers in the left margin before the ':' are node number (index of node in

not sure this I'd call this node number (since node number is the index that's used as an identifier). Just say node index inside basic block

> Source/JavaScriptCore/dfg/DFGGraph.h:1007
> +    void incPhase() { m_prefix.phaseNumber++; }

maybe call this "nextPhase" or "startingNextPhase"?
Comment 3 Mark Lam 2019-08-13 16:30:47 PDT
Comment on attachment 376221 [details]
proposed patch.

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

>> Source/JavaScriptCore/ChangeLog:46
>> +        The numbers in the left margin before the ':' are node number (index of node in
> 
> not sure this I'd call this node number (since node number is the index that's used as an identifier). Just say node index inside basic block

There's already a m_index and index() in DFGNode, but I'll just clarify that I mean a different index here.

>> Source/JavaScriptCore/dfg/DFGGraph.h:1007
>> +    void incPhase() { m_prefix.phaseNumber++; }
> 
> maybe call this "nextPhase" or "startingNextPhase"?

Will rename to nextPhase.
Comment 4 Mark Lam 2019-08-13 16:32:06 PDT
Thanks for the review.  Landed in r248642: <http://trac.webkit.org/r248642>.
Comment 5 Radar WebKit Bug Importer 2019-08-13 16:33:25 PDT
<rdar://problem/54279703>