Samsung / walrus

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

Remove bytecode duplication #232

Closed GorogPeter closed 6 months ago

GorogPeter commented 6 months ago

I found that the parser duplicates move instructions at the end of blocks in some cases.

For example:

(module
  (func (result i32 i32 i32)
    (block (result i32 i32 i32)
      i32.const 1
      i32.const 2
      i32.const 3
    )
  )
)

Results this:

required stack size: 48 bytes
stack: [(constant 1, i32, pos 0) (constant 2, i32, pos 8) (constant 3, i32, pos 16) ....]
bytecode size: 168 bytes

     0 const32 dstOffset: 0 value: 1
    16 const32 dstOffset: 8 value: 2
    32 const32 dstOffset: 16 value: 3
    48 move32 srcOffset: 0 dstOffset: 24 
    64 move32 srcOffset: 8 dstOffset: 32 
    80 move32 srcOffset: 16 dstOffset: 40 
    96 move32 srcOffset: 16 dstOffset: 40 
   112 move32 srcOffset: 8 dstOffset: 32 
   128 move32 srcOffset: 0 dstOffset: 24 
   144 end resultOffsets: 24 32 40

The duplication occures in keepBlockResultsIfNeeds(...) function, here and here.

By removing the first one (with the if statement), it does not duplicate the bytecode:

required stack size: 48 bytes
stack: [(constant 1, i32, pos 0) (constant 2, i32, pos 8) (constant 3, i32, pos 16) ....]
bytecode size: 120 bytes

     0 const32 dstOffset: 0 value: 1
    16 const32 dstOffset: 8 value: 2
    32 const32 dstOffset: 16 value: 3
    48 move32 srcOffset: 16 dstOffset: 40 
    64 move32 srcOffset: 8 dstOffset: 32 
    80 move32 srcOffset: 0 dstOffset: 24 
    96 end resultOffsets: 24 32 40

It also passes the tests and the benchmark tests as well.

But unfortunately, I'm not completely aware of the impact of this change on Walrus. Could someone help me, please?

ksh8281 commented 6 months ago

IMO you found good point!