Bug 200693

Summary: Add phase, block, and node numbers to left margin of DFG graph dumps.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. saam: review+

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>