[Merged by Bors] - Implement instruction flowgraph generator #2422

HalidOdat · 2022-11-08T14:45:14Z

This PR is a WIP implementation of a vm instruction flowgraph generator

This aims to make the vm easier to debug and understand for both newcomers and experienced devs.

For example if we have the following code:

let i = 0;
while (i < 10) {
    if (i == 3) {
        break;
    }
    i++;
}

It generates the following instructions (which is hard to read, especially jumps):

----------------------Compiled Output: '<main>'-----------------------
Location  Count   Opcode                     Operands

000000    0000    PushZero
000001    0001    DefInitLet                 0000: 'i'
000006    0002    LoopStart
000007    0003    LoopContinue
000008    0004    GetName                    0000: 'i'
000013    0005    PushInt8                   10
000015    0006    LessThan
000016    0007    JumpIfFalse                78
000021    0008    PushDeclarativeEnvironment 0, 1
000030    0009    GetName                    0000: 'i'
000035    0010    PushInt8                   3
000037    0011    Eq
000038    0012    JumpIfFalse                58
000043    0013    PushDeclarativeEnvironment 0, 0
000052    0014    Jump                       78
000057    0015    PopEnvironment
000058    0016    GetName                    0000: 'i'
000063    0017    IncPost
000064    0018    RotateRight                2
000066    0019    SetName                    0000: 'i'
000071    0020    Pop
000072    0021    PopEnvironment
000073    0022    Jump                       7
000078    0023    LoopEnd

Literals:
    <empty>

Bindings:
    0000: i

Functions:
    <empty>

And the flow graph is generated:

The beginning of the function is marked by the start node (in green) and end (in red). In branching the "yes" branch is marked in green and "no" in red.

This only generates in graphviz format (a widely used format) but it would be nice to also generate to a format that mermaid.js can understand and that could be put in articles boa-dev/boa-dev.github.io#26

TODO:

github-actions · 2022-11-08T14:51:34Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	93,831	93,831	0
Passed	69,620	69,620	0
Ignored	18,472	18,472	0
Failed	5,739	5,739	0
Panics	0	0	0
Conformance	74.20%	74.20%	0.00%

codecov · 2022-11-08T14:55:47Z

Codecov Report

Merging #2422 (c6e956f) into main (20db887) will decrease coverage by 0.94%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2422      +/-   ##
==========================================
- Coverage   52.58%   51.63%   -0.95%     
==========================================
  Files         329      336       +7     
  Lines       34905    35541     +636     
==========================================
- Hits        18354    18353       -1     
- Misses      16551    17188     +637

Impacted Files	Coverage Δ
boa_cli/src/main.rs	`0.00% <ø> (ø)`
boa_engine/src/vm/flowgraph/color.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/flowgraph/edge.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/flowgraph/graph.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/flowgraph/mod.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/flowgraph/node.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/mod.rs	`47.97% <ø> (ø)`
boa_engine/src/vm/opcode/mod.rs	`57.14% <ø> (ø)`
boa_ast/src/visitor.rs	`10.58% <0.00%> (-0.59%)`	⬇️
boa_engine/src/object/mod.rs	`45.98% <0.00%> (-0.11%)`	⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

jedel1043 · 2022-11-08T15:55:16Z

Very nice work! This is a huge improvement for debugging. I have just a small nitpick though. Can it be generated vertically instead of horizontally? I think it will read better that way.

HalidOdat · 2022-11-08T16:04:51Z

Very nice work! This is a huge improvement for debugging. I have just a small nitpick though. Can it be generated vertically instead of horizontally? I think it will read better that way.

Yes, that is configurable (I did it horizontaly because it looked better on github comments) I plan to add an option for that

And there are other generation modes, where you can better see the loop:

HalidOdat · 2022-11-08T16:08:29Z

Hmmm.. looking at the flowgraph it looks like there is a PopEnvironment which will never be executed (since nothing "flows" into it). I'm not sure if it is a bug on my part or in the bytecompiler, if it is on the bytecompiler this is already proving to be useful 😆

jedel1043 · 2022-11-08T16:18:15Z

Hmmm.. looking at the flowgraph it looks like there is a PopEnvironment which will never be executed (since nothing "flows" into it). I'm not sure if it is a bug on my part or in the bytecompiler, if it is on the bytecompiler this is already proving to be useful 😆

That's definitely a bug! JumpIfFalse is jumping to instruction 0x58, skipping the PopEnvironment, and the plain Jump also skips it by jumping to the end of the function.

HalidOdat · 2022-11-08T16:45:32Z

I guess, I'll create an issue about it :)

jedel1043 · 2022-11-08T16:52:01Z

What if we coloured pairs of PushEnvironmnent and PopEnvironment with the same colour, and changed the colour for each pair? That way we can identify if environments are being handled correctly.

HalidOdat · 2022-11-08T17:27:45Z

Here is a implementation of that, it assumes that the generated push and pop instructions always are in that order., which should be the case generaly (if not always), unless there are some wierd jumping back and forth.
Otherwise we need to hook into the bytecompiler, because we lose context after compilation.

Also included the opcode location prefix on all nodes.

Here are the graphs generated:

This is all temporary code, will try to create nice options for generating dot and mermaid.js formats tomorrow :)

Razican

This is extremely good. Just seeing the graphs I'm starting to understand the VM, which is already a huge step for me 😅

And it's already showing how this can spot problems!

The code needs a bit of documentation, polishing and maybe the color constant could be reviewed to be more configurable, but it's already in pretty good shape.

Thank you!

jasonwilliams · 2022-11-09T08:37:38Z

This is great! And definitely helps with seeing how things work with the VM a lot.

I’m very interested in mermaid integration as that’s supported on GitHub and VSCode now so should make collaboration easier. It should save on needing to install an additional viewer. The syntax looks similar but I don’t know if there’s any limitations.

HalidOdat · 2022-11-09T14:19:08Z

I still need to add many things but we can generate graphviz and mermaid diagrams

Since mermaid diagrams are supported by github comments (flipped it horizontally because its to big):

graph LR
    __main___i_0[PushZero]
    __main___i_1[DefInitLet 'i']
    __main___i_6[LoopStart]
    __main___i_7[LoopContinue]
    __main___i_8[GetName 'i']
    __main___i_13[PushInt8 10]
    __main___i_15[LessThan]
    __main___i_16{JumpIfFalse}
    __main___i_21[PushDeclarativeEnvironment 0, 1]
    __main___i_30[GetName 'i']
    __main___i_35[PushInt8 3]
    __main___i_37[Eq]
    __main___i_38{JumpIfFalse}
    __main___i_43[PushDeclarativeEnvironment 0, 0]
    __main___i_52{Jump}
    __main___i_57[PopEnvironment]
    __main___i_58[GetName 'i']
    __main___i_63[IncPost]
    __main___i_64[RotateRight 2]
    __main___i_66[SetName 'i']
    __main___i_71[Pop]
    __main___i_72[PopEnvironment]
    __main___i_73{Jump}
    __main___i_78[LoopEnd]
    __main___i_79{End}
    __main___i_80{Start}
    __main___i_0 -->| | __main___i_1
    __main___i_1 -->| | __main___i_6
    __main___i_6 -->| | __main___i_7
    __main___i_7 -->| | __main___i_8
    __main___i_8 -->| | __main___i_13
    __main___i_13 -->| | __main___i_15
    __main___i_15 -->| | __main___i_16
    __main___i_16 -->| YES| __main___i_78
    __main___i_16 -->| NO| __main___i_21
    __main___i_21 -->| | __main___i_30
    __main___i_30 -->| | __main___i_35
    __main___i_35 -->| | __main___i_37
    __main___i_37 -->| | __main___i_38
    __main___i_38 -->| YES| __main___i_58
    __main___i_38 -->| NO| __main___i_43
    __main___i_43 -->| | __main___i_52
    __main___i_52 -->| | __main___i_78
    __main___i_57 -->| | __main___i_58
    __main___i_58 -->| | __main___i_63
    __main___i_63 -->| | __main___i_64
    __main___i_64 -->| | __main___i_66
    __main___i_66 -->| | __main___i_71
    __main___i_71 -->| | __main___i_72
    __main___i_72 -->| | __main___i_73
    __main___i_73 -->| | __main___i_7
    __main___i_78 -->| | __main___i_79
    __main___i_80 -->| | __main___i_0

boa_engine/src/vm/code_block.rs

Razican

Still a lot to review, but it looks great :) I added some comments related to documentation and Clone implementations.

boa_cli/src/main.rs

boa_engine/src/vm/graph.rs

HalidOdat · 2022-11-13T18:17:15Z

Added documentation in debugging.md file, you can view how it looks here: https://github.com/boa-dev/boa/blob/60d33337d322b7cf328b9237cf67a172a24d2d20/docs/debugging.md#instruction-flowgraph

Razican

LGTM :)

Razican · 2022-11-22T18:32:17Z

bors r+

bors · 2022-11-22T18:32:21Z

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors · 2022-11-22T18:33:25Z

👎 Rejected by PR status

Razican · 2022-11-22T20:04:41Z

bors r+

This PR is a WIP implementation of a vm instruction flowgraph generator This aims to make the vm easier to debug and understand for both newcomers and experienced devs. For example if we have the following code: ```js let i = 0; while (i < 10) { if (i == 3) { break; } i++; } ``` It generates the following instructions (which is hard to read, especially jumps): <details> ``` ----------------------Compiled Output: '<main>'----------------------- Location Count Opcode Operands 000000 0000 PushZero 000001 0001 DefInitLet 0000: 'i' 000006 0002 LoopStart 000007 0003 LoopContinue 000008 0004 GetName 0000: 'i' 000013 0005 PushInt8 10 000015 0006 LessThan 000016 0007 JumpIfFalse 78 000021 0008 PushDeclarativeEnvironment 0, 1 000030 0009 GetName 0000: 'i' 000035 0010 PushInt8 3 000037 0011 Eq 000038 0012 JumpIfFalse 58 000043 0013 PushDeclarativeEnvironment 0, 0 000052 0014 Jump 78 000057 0015 PopEnvironment 000058 0016 GetName 0000: 'i' 000063 0017 IncPost 000064 0018 RotateRight 2 000066 0019 SetName 0000: 'i' 000071 0020 Pop 000072 0021 PopEnvironment 000073 0022 Jump 7 000078 0023 LoopEnd Literals: <empty> Bindings: 0000: i Functions: <empty> ``` </details> And the flow graph is generated: ![flowgraph](https://user-images.githubusercontent.com/8566042/200589387-40b36ad7-d2f2-4918-a3e4-5a8fa5eee89b.png) The beginning of the function is marked by the `start` node (in green) and end (in red). In branching the "yes" branch is marked in green and "no" in red. ~~This only generates in [graphviz format](https://en.wikipedia.org/wiki/DOT_(graph_description_language)) (a widely used format) but it would be nice to also generate to a format that `mermaid.js` can understand and that could be put in articles boa-dev/boa-dev.github.io#26 TODO: - [x] Generate graphviz format - [x] Generate mermaid format - [x] Programmatically generate colors push and pop env instructions - [x] Display nested functions in sub-sub-graphs. - [x] Put under a feature (`"flowgraph"`) - [x] Handle try/catch, switch instructions - [x] CLI option for configuring direction of flow (by default it is top down) - [x] Handle `Throw` instruction (requires keeping track of try blocks) - [x] Documentation - [x] Prevent node name collisions (functions with the same name)

bors · 2022-11-22T20:24:58Z

Pull request successfully merged into main.

Build succeeded:

HalidOdat added enhancement New feature or request vm Issues and PRs related to the Boa Virtual Machine. labels Nov 8, 2022

HalidOdat changed the title ~~Implement instruction to flowgraph visualizer~~ Implement instruction flowgraph generator Nov 8, 2022

HalidOdat mentioned this pull request Nov 8, 2022

JumpIfFalse skips over PopEnvironment instruction #2424

Closed

Razican reviewed Nov 8, 2022

View reviewed changes

HalidOdat force-pushed the feature/instruction-flowgraph branch from 8182cc6 to 467f143 Compare November 9, 2022 14:09

Razican reviewed Nov 10, 2022

View reviewed changes

boa_engine/src/vm/code_block.rs Outdated Show resolved Hide resolved

HalidOdat added 9 commits November 10, 2022 17:07

Implement generic graph with graphviz format

5bda107

Generate mermaid graphs

0a87747

generate colored nodes and edges

c8426b5

Simplify adding nodes and edges

6acf808

Add subgraphs

6b19f04

Add recursive subgraphs

6315aef

Move to_graph to graph module

2b1b457

Add CLI option for generating flowgraphs

130d3eb

Put under feature-gate

6e945a1

HalidOdat force-pushed the feature/instruction-flowgraph branch from 8ac3b2d to 6e945a1 Compare November 10, 2022 16:24

HalidOdat added 2 commits November 10, 2022 19:06

Programmatically generate colors

0676727

Make Return instruction point to End

b4fc149

Include opcode locations

fe440a3

Razican requested review from Razican, jasonwilliams, RageKnify and raskad November 11, 2022 10:34

Razican added this to the v0.17.0 milestone Nov 11, 2022

Razican reviewed Nov 11, 2022

View reviewed changes

HalidOdat added 5 commits November 13, 2022 17:11

Documentation

12757d9

Document mermaid block fix

1dd9022

Revent node name collisions

050fc64

Split into sub-modules

b0ca0fd

Refactor Color

5f48fb2

HalidOdat requested a review from Razican November 13, 2022 18:11

Add documentation to flowgraph feature

60d3333

RageKnify approved these changes Nov 13, 2022

View reviewed changes

Run rrettier on debugging.md

c27e382

jasonwilliams approved these changes Nov 19, 2022

View reviewed changes

Merge branch 'main' into feature/instruction-flowgraph

a56b0ee

Razican approved these changes Nov 22, 2022

View reviewed changes

Fix clippy error

c6e956f

bors bot changed the title ~~Implement instruction flowgraph generator~~ [Merged by Bors] - Implement instruction flowgraph generator Nov 22, 2022

bors bot closed this Nov 22, 2022

bors bot deleted the feature/instruction-flowgraph branch November 22, 2022 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Implement instruction flowgraph generator #2422

[Merged by Bors] - Implement instruction flowgraph generator #2422

HalidOdat commented Nov 8, 2022 •

edited

Loading

github-actions bot commented Nov 8, 2022 •

edited

Loading

codecov bot commented Nov 8, 2022 •

edited

Loading

jedel1043 commented Nov 8, 2022

HalidOdat commented Nov 8, 2022 •

edited

Loading

HalidOdat commented Nov 8, 2022 •

edited

Loading

jedel1043 commented Nov 8, 2022

HalidOdat commented Nov 8, 2022

jedel1043 commented Nov 8, 2022 •

edited

Loading

HalidOdat commented Nov 8, 2022 •

edited

Loading

Razican left a comment

jasonwilliams commented Nov 9, 2022 •

edited

Loading

HalidOdat commented Nov 9, 2022

Razican left a comment

HalidOdat commented Nov 13, 2022

Razican left a comment

Razican commented Nov 22, 2022

bors bot commented Nov 22, 2022

bors bot commented Nov 22, 2022

Razican commented Nov 22, 2022

bors bot commented Nov 22, 2022

[Merged by Bors] - Implement instruction flowgraph generator #2422

[Merged by Bors] - Implement instruction flowgraph generator #2422

Conversation

HalidOdat commented Nov 8, 2022 • edited Loading

github-actions bot commented Nov 8, 2022 • edited Loading

Test262 conformance changes

codecov bot commented Nov 8, 2022 • edited Loading

Codecov Report

jedel1043 commented Nov 8, 2022

HalidOdat commented Nov 8, 2022 • edited Loading

HalidOdat commented Nov 8, 2022 • edited Loading

jedel1043 commented Nov 8, 2022

HalidOdat commented Nov 8, 2022

jedel1043 commented Nov 8, 2022 • edited Loading

HalidOdat commented Nov 8, 2022 • edited Loading

Razican left a comment

Choose a reason for hiding this comment

jasonwilliams commented Nov 9, 2022 • edited Loading

HalidOdat commented Nov 9, 2022

Razican left a comment

Choose a reason for hiding this comment

HalidOdat commented Nov 13, 2022

Razican left a comment

Choose a reason for hiding this comment

Razican commented Nov 22, 2022

bors bot commented Nov 22, 2022

bors bot commented Nov 22, 2022

Razican commented Nov 22, 2022

bors bot commented Nov 22, 2022

HalidOdat commented Nov 8, 2022 •

edited

Loading

github-actions bot commented Nov 8, 2022 •

edited

Loading

codecov bot commented Nov 8, 2022 •

edited

Loading

HalidOdat commented Nov 8, 2022 •

edited

Loading

HalidOdat commented Nov 8, 2022 •

edited

Loading

jedel1043 commented Nov 8, 2022 •

edited

Loading

HalidOdat commented Nov 8, 2022 •

edited

Loading

jasonwilliams commented Nov 9, 2022 •

edited

Loading