Skip to content

Non-nullable locals mode not producing non-nullable locals #3953

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

Closed
askeksa-google opened this issue Jun 25, 2021 · 0 comments · Fixed by #3955
Closed

Non-nullable locals mode not producing non-nullable locals #3953

askeksa-google opened this issue Jun 25, 2021 · 0 comments · Fixed by #3955

Comments

@askeksa-google
Copy link

I have been trying out the --enable-gc-nn-locals flag on some output modules from dart2wasm. I was surprised to find that the optimized Binaryen output when using non-nullable locals was bigger than the one using only nullable locals, even though for the original modules, the one using non-nullable locals was smaller.

I looked at the output and noticed a few things:

  • The optimized non-nullable version doesn't actually contain any non-nullable locals. This is the case even for a non-optimizing wasm-to-wat pass-through.
  • The code contains many superfluous ref.as_non_null, i.e. on a non-nullable value or as input to something accepting a nullable value.

It looks like the code was processed with non-nullable locals in mind, but for the output, the locals somehow became nullable and all the ref.as_non_null were inserted to make the types match up.

Since at least some of these could potentially trap, it would seem that removing them is only valid with the --ignore-implicit-traps option. Thus, a fix for #3934 could be valuable for doing a fair comparison between having non-nullable locals and not.

kripken added a commit that referenced this issue Jun 25, 2021
kripken added a commit that referenced this issue Jul 2, 2021
We only tested that feature on the text format. For binary support, the reader needs
to know that the feature is enabled, so that it allows non-nullable locals in that
case (i.e., does not apply the workarounds to remove them).

Fixes #3953
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

Successfully merging a pull request may close this issue.

1 participant