Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasm-opt crash: "Fatal: Unsupported instruction for Flatten: BrOn" #7053

Open
osa1 opened this issue Nov 5, 2024 · 3 comments
Open

wasm-opt crash: "Fatal: Unsupported instruction for Flatten: BrOn" #7053

osa1 opened this issue Nov 5, 2024 · 3 comments

Comments

@osa1
Copy link
Contributor

osa1 commented Nov 5, 2024

wasm-opt crashes when I try to optimize the Wasm file in the zip file (attached):

$ wasm-opt -g --all-features --closed-world --traps-never-happen --type-unfinalizing -Os --type-ssa --gufa -Os --type-merging -O4 --type-finalizing test.wasm -o test.opt.wasm
Fatal: Unsupported instruction for Flatten: BrOn
Fatal: Unsupported instruction for Flatten: BrOn

I'm trying with the current main branch (320867a):

$ wasm-opt --version
wasm-opt version 120 (version_117-528-g320867a7c)

Update: it also happens with wasm-opt version 119 (93883fd).

test.wasm.zip

@kripken
Copy link
Member

kripken commented Nov 5, 2024

Looks like this is #6989 (comment) - we don't yet support -O4 on all instructions, including BrOn.

Was there a specific reason you wanted that instead of -O3?

@osa1
Copy link
Contributor Author

osa1 commented Nov 6, 2024

Was there a specific reason you wanted that instead of -O3?

-O3 is unable to hoist the field load inside the loop in this function:

(func $foo1 (;299;) (param $var0 (ref $OneByteString)) (result i64)
  (local $var1 i64)
  (local $var2 i64)
  (local $var3 (ref $Array<WasmI8>))
  loop $label0  
    local.get $var1
    local.get $var0
    struct.get $OneByteString $field2
    local.tee $var3                 
    array.len
    i64.extend_i32_u
    i64.lt_s
    if
      local.get $var3
      local.get $var1
      i32.wrap_i64
      array.get_u $Array<WasmI8>
      i64.extend_i32_u
      i64.eqz
      if
        local.get $var3            
        i32.const 1000000000
        array.get_u $Array<WasmI8> 
        i64.extend_i32_u
        local.get $var2
        i64.add
        local.set $var2
      end
      local.get $var1
      i64.const 1
      i64.add
      local.set $var1
      br $label0
    end
  end $label0
  local.get $var2)

Here var0 never changes, and the field field2 is immutable, so the struct.get could be hoisted outside of the loop and the field would be loaded only once, instead of in each iteration. I wanted to see if -O4 does this.

@kripken
Copy link
Member

kripken commented Nov 6, 2024

Ah, then -O4 does not do that either. We do have an licm pass, but it is disabled by default in all modes (since it tends to increase code size, and VMs do licm themselves anyhow). But you can experiment with it by adding --licm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants