binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

RemoveUnusedBrs causes ineffective code

Open xuruiyang2002 opened this issue 6 months ago • 1 comments

Given the following code:

(module
  (import "External" "external_function" (func $external_function))
  (func $_start (param $0 i32) (param $1 i32)
    (local $2 i32) (local $3 i32)
    i32.const 587
    i32.load8_u
    local.set $0
    i32.const 24
    local.set $1
    local.get $0
    local.get $1
    i32.shl
    local.get $1
    i32.shr_s
    local.set $0
    i32.const 587
    i32.load8_u
    local.set $1
    i32.const 24
    local.set $2
    local.get $1
    local.get $2
    i32.shl
    local.get $2
    i32.shr_s
    local.set $1
    i32.const 587
    i32.load8_u
    local.set $2
    i32.const 24
    local.set $3
    local.get $2
    local.get $3
    i32.shl
    local.get $3
    i32.shr_s
    local.set $2
    local.get $1
    local.get $2
    i32.ge_s
    local.set $1
    block  ;; label = @1
      local.get $1
      br_if 0 (;@1;)
      call $external_function
    end
    local.get $0
    i32.const 0
    call $foo
    drop
    unreachable)
  (func $foo (param $0 i32) (param $1 i32) (result i32)
    unreachable)
  (memory $0 258 258)
  (export "_start" (func $_start)))

For wasm-opt (c91c052), -O3 -sp=remove-unused-brs produces:

 (func $_start (param $0 i32) (param $1 i32)
  (drop
   (i32.load8_u
    (i32.const 587)
   )
  )
  (unreachable)
 )

while -O3 produces counter-intuitive code:

 (func $_start (param $0 i32) (param $1 i32)
  (if
   (i32.lt_s
    (local.tee $1
     (i32.extend8_s
      (i32.load8_s
       (i32.const 587)
      )
     )
    )
    (local.get $1)
   )
   (then
    (call $external_function)
   )
  )
  (unreachable)
 )

The RemoveUnusedBrs takes lots efforts to optimize the control flow. However, it causes poor code, which defies our expectations.

xuruiyang2002 avatar Jun 17 '25 15:06 xuruiyang2002

Below is the change made by RemoveUnusedBrs:

Image

I think it is the restructureIf that causes degradation, for it converts the block with br_if structure into if statement. What's more, currently optimization for if's condition is limited (another case demonstrates this limitation is #7659 ), thus leading to this counter-intuitive case.

As you can see in this case, is it really beneficial to always do restructureIf? Or maybe a strategy to decide is needed, thus can exploit the structure more rationale.

xuruiyang2002 avatar Jun 18 '25 03:06 xuruiyang2002