binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Flatten: Support `br_on_null`, `br_on_non_null`, `br_on_cast`, `br_on_cast_fail`

Open osa1 opened this issue 4 months ago • 1 comments

Fixes #6989.

With this we're able to use -O4 to optimize dart2wasm programs. I tried this on a few benchmarks, largest one being a 37M .wasm, and it worked fine.

Flattening transformations done:

(br_on_null $label <flat expr>)

==>

Locals:
  (local $nullableTemp <flat expr type>)
  (local $isNull i32)

Preludes:
  (local.set $nullableTemp <flat expr>)
  (local.set $isNull (ref_is_null (local.get $nullableTemp)
  (br_if $label (local.get $isNull))

Expr:
  (ref_as_non_null (local.get $nullableTemp)
(br_on_non_null $label <flat expr>)

==>

Locals:
  (local $nullableTemp <flat expr type>)
  (local $isNull i32)
  (local $isNotNull i32)

Preludes:
  (local.set $nullableTemp <flat expr>)
  (local.set $isNull (ref_is_null (local.get $nullableTemp)
  (local.set $isNotNull (i32.eqz (local.get $isNull)))

Expr:
  (if (local.get $isNotNull)
    (local.set $labelLocal (ref_as_non_null (local.get $nullableTemp)))
    (br $label))
(br_on_cast $label $s $t <flat expr>)

==> 

Locals:
  (local $source $s)
  (local $typeTest i32)
  (local $failTemp ($s \ $t))

Preludes:
  (local.set $source <flat expr>)
  (local.set $typeTest (ref_test (local.get $source) $t))

  (if (local.get $typeTest)
    (local.set $labelLocal (ref_cast (local.get $source) $s $t))
    (br $label))

  ;; If `$s \ $t` is non-nullable:
  (local.set $failTemp (ref_as_non_null (local.get $source)))
  ;; Otherwise:
  (local.set $failTemp (local.get $source))

Expr:
  (local.get $failTemp)
(br_on_cast_failure $label $s $t <flat expr>)

==>

Locals:
  (local $source $s)
  (local $target $t)
  (local $typeTest i32)
  (local $typeTestFail i32)
  (local $failTemp ($s \ $t))

Preludes:
  (local.set $source <flat expr>)
  (local.set $typeTest (ref_test (local.get $source) $t))
  (local.set $typeTestFail (i32.eqz (local.get $typeTest)))

  (if (local.get $typeTestFail)
    ;; If `$s \ $t` is non-nullable:
    (local.set $labelLocal (ref_as_non_null (local.get $source)))
    ;; Otherwise
    (local.set $labelLocal (local.get $source))
  )

  (local.set $target (ref_cast (local.get $source) $s $t))

Expr:
  (local.get $target)

osa1 avatar Oct 02 '25 08:10 osa1

@tlively @kripken This is ready for reviews, but the testing here may not be sufficient. Because I use ref_as_non_null and ref_cast in the lowered code, when done incorectly these instructions can make the Wasm module valid but just do ref_as_non_null on null references. This happened in a previous version of this PR and I only caught it in dart2wasm.

I'm not sure how to test for this as we would need to run the lowered program. For now I tested this on a few dart2wasm benchmarks (largest one being a 37M .wasm) and it works fine.

Any suggestions on how to improve testing here?

I suggest reviewing the transformations shown in the PR description first. If those transformations are not right then the code also won't be right. (unless I made a mistake while transcribing)

osa1 avatar Oct 03 '25 08:10 osa1