Samsung / walrus

WebAssembly Lightweight RUntime
Apache License 2.0
40 stars 10 forks source link

Improve debug mode #111

Closed GorogPeter closed 1 year ago

GorogPeter commented 1 year ago

Now the debug mode is better arranged, easier to examine. Dump() functions in ByteCode.h have been divided into getName() and printExtra() functions.

Old debug mode:

function type 0x556f7987a5d0
requiredStackSize 16, requiredStackSizeDueToLocal 0
0: I32Eqz src: 0 dst: 8
16: jump_if_true srcOffset: 8 dst: 64
32: const32 dstOffset: 8 value: 42
48: move64 srcOffset: 8 dstOffset: 0 
64: end resultOffsets: 0  
function type 0x556f7987a5d0
requiredStackSize 32, requiredStackSizeDueToLocal 8
0: I32Eqz src: 0 dst: 16
16: jump_if_true srcOffset: 16 dst: 128
32: const32 dstOffset: 24 value: 1
48: I32Eqsrc1: 0 src2: 24 dst: 16
64: jump_if_true srcOffset: 16 dst: 176
80: const32 dstOffset: 16 value: 7
96: move64 srcOffset: 16 dstOffset: 8 
112: jump dst: 208
128: const32 dstOffset: 16 value: 42
144: move64 srcOffset: 16 dstOffset: 8 
160: jump dst: 208
176: const32 dstOffset: 16 value: 99
192: move64 srcOffset: 16 dstOffset: 8 
208: end resultOffsets: 8  
invoke block1(0) expect value(0) (line: 35) : OK
invoke block1(1) expect value(42) (line: 36) : OK
invoke block2(0) expect value(42) (line: 38) : OK
invoke block2(1) expect value(99) (line: 39) : OK
invoke block2(2) expect value(7) (line: 40) : OK

New debug mode:

| function type                   : 0x55f83d3bc5d0
| required stack size             :       16 bytes
| required stack size due to local:        0 bytes
| bytecode size                   :      122 bytes
|
|  address        | size | opcode          | note      
|-----------------+------+-----------------+-----------------
|               0 |   24 | I32Eqz          | src: 0 dst: 8
|              24 |   24 | jump_if_true    | srcOffset: 8 dst: 96
|              48 |   24 | const32         | dstOffset: 8 value: 42
|              72 |   24 | move64          | srcOffset: 8 dstOffset: 0 
|              96 |   26 | end             | resultOffsets: 0 
|-----------------+------+-----------------+-----------------

| function type                   : 0x55f83d3bc5d0
| required stack size             :       32 bytes
| required stack size due to local:        8 bytes
| bytecode size                   :      338 bytes
|
|  address        | size | opcode          | note      
|-----------------+------+-----------------+-----------------
|               0 |   24 | I32Eqz          | src: 0 dst: 16
|              24 |   24 | jump_if_true    | srcOffset: 16 dst: 192
|              48 |   24 | const32         | dstOffset: 24 value: 1
|              72 |   24 | I32Eq           | src1: 0 src2: 24 dst: 16
|              96 |   24 | jump_if_true    | srcOffset: 16 dst: 264
|             120 |   24 | const32         | dstOffset: 16 value: 7
|             144 |   24 | move64          | srcOffset: 16 dstOffset: 8 
|             168 |   24 | jump            | dst: 312
|             192 |   24 | const32         | dstOffset: 16 value: 42
|             216 |   24 | move64          | srcOffset: 16 dstOffset: 8 
|             240 |   24 | jump            | dst: 312
|             264 |   24 | const32         | dstOffset: 16 value: 99
|             288 |   24 | move64          | srcOffset: 16 dstOffset: 8 
|             312 |   26 | end             | resultOffsets: 8 
|-----------------+------+-----------------+-----------------

invoke block1(0) expect value(0) (line: 35) : OK
invoke block1(1) expect value(42) (line: 36) : OK
invoke block2(0) expect value(42) (line: 38) : OK
invoke block2(1) expect value(99) (line: 39) : OK
invoke block2(2) expect value(7) (line: 40) : OK
zherczeg commented 1 year ago

It is quite a sophisticated output, but do we need it?

GorogPeter commented 1 year ago

I think, with this patch, it's far easier to examine the bytecode. Shall I ask our colleagues, whether the patch is useful for them?

kulcsaradam commented 1 year ago

I believe it is a bit easier on the eyes, and when we examine larger blocks of generated bytecodes it could be more readable than the previous version. I would like this patch to land if possible.

GorogPeter commented 1 year ago

Now it looks like this:

required stack size: 32 bytes
required stack size due to local: 16 bytes
bytecode size: 186 bytes

     0 GlobalGet64 index: 0
    16 Move64 srcOffset: 16 dstOffset: 8 
    32 Const32 dstOffset: 24 value: 4
    48 I32Mul src1: 0 src2: 24 dst: 16
    64 Load32 srcOffset: 16 dstOffset: 16 
    80 Call index: 0 paramOffsets: 16  resultOffsets: 
   106 Const32 dstOffset: 24 value: 1
   122 I32Add src1: 0 src2: 24 dst: 0
   138 I32Ne src1: 0 src2: 8 dst: 16
   154 JumpIfTrue srcOffset: 16 dst: 32
   170 End resultOffsets: 

required stack size: 40 bytes
required stack size due to local: 16 bytes
bytecode size: 254 bytes

     0 GlobalGet64 index: 0
    16 Move64 srcOffset: 16 dstOffset: 8 
    32 Const32 dstOffset: 24 value: 4
    48 I32Mul src1: 0 src2: 24 dst: 16
    64 I32Sub src1: 8 src2: 0 dst: 24
    80 Store32 src0Offset: 16 src1Offset: 24 
    96 Const32 dstOffset: 24 value: 1
   112 I32Add src1: 0 src2: 24 dst: 0
   128 I32Ne src1: 0 src2: 8 dst: 16
   144 JumpIfTrue srcOffset: 16 dst: 32
   160 Const32 dstOffset: 16 value: 0
   176 Const32 dstOffset: 24 value: 0
   192 GlobalGet64 index: 0
   208 Call index: 1 paramOffsets: 16 24 32  resultOffsets: 
   238 End resultOffsets: 

required stack size: 16 bytes
required stack size due to local: 0 bytes
bytecode size: 34 bytes

     0 Load32 srcOffset: 0 dstOffset: 8 
    16 End resultOffsets: 8 

invoke read(0) expect value(0) (line: 212) : OK
invoke read(4) expect value(1) (line: 213) : OK
invoke read(8) expect value(2) (line: 214) : OK
GorogPeter commented 1 year ago

Now, the debug mode looks like this:

required stack size: 40 bytes
required stack size due to local: 16 bytes
bytecode size: 254 bytes

     0 global.get64 index: 0
    16 move64 srcOffset: 16 dstOffset: 8
    32 const32 dstOffset: 24 value: 4
    48 I32Mul src1: 0 src2: 24 dst: 16
    64 I32Sub src1: 8 src2: 0 dst: 24
    80 store32 src0Offset: 16 src1Offset: 24
    96 const32 dstOffset: 24 value: 1
   112 I32Add src1: 0 src2: 24 dst: 0
   128 I32Ne src1: 0 src2: 8 dst: 16
   144 jump_if_true srcOffset: 16 dst: 32
   160 const32 dstOffset: 16 value: 0
   176 const32 dstOffset: 24 value: 0
   192 global.get64 index: 0
   208 call index: 1 paramOffsets: 16 24 32  resultOffsets:
   238 end resultOffsets:

required stack size: 16 bytes
required stack size due to local: 0 bytes
bytecode size: 34 bytes

     0 load32 srcOffset: 0 dstOffset: 8
    16 end resultOffsets: 8

invoke read(0) expect value(0) (line: 212) : OK
invoke read(4) expect value(1) (line: 213) : OK
invoke read(8) expect value(2) (line: 214) : OK
clover2123 commented 1 year ago

@GorogPeter Is this patch done or any updates left?

GorogPeter commented 1 year ago

@clover2123 Yes, it's done.