Skip to content

Calculate wasm "env" imports at compile time#9447

Merged
kripken merged 18 commits into
incomingfrom
ref
Sep 17, 2019
Merged

Calculate wasm "env" imports at compile time#9447
kripken merged 18 commits into
incomingfrom
ref

Conversation

@kripken

@kripken kripken commented Sep 17, 2019

Copy link
Copy Markdown
Member

This moves that code from runtime addition of fields to the env, to compile-time addition and emitting of the final import object as a whole.

This has no significant functional change. It just makes the emitted code smaller and simpler. This also simplifies the overall model, and will allow further refactoring to simplify the dynamic linking code (which currently has its own computation of the wasm imports), leaving that for a later PR to keep this one as small as possible. This will also help with the code splitting experiment in the future.

  • This fixes the acorn parser passes to handle wasm import fields that are not just names, as now they can also be numbers or more complex expressions (the stuff we compute at compile time now may include such things).

  • The metadce tests get lots of updates, but there is no substance to them: the tests look at the initial wasm import object, before we added more fields at runtime. We don't add the fields at runtime anymore, so the tests can see those extra fields now, but they were always there, just the tests didn't notice them before. So there is no increase in the number of wasm imports, it's the same.

  • When testing this I also noticed running random tests was broken on fastcomp, we were just missing an if in runner.py, I added that here.

@sbc100 sbc100 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good direction in general.

It also reminds me that I think we should add some expected output tests so that when changes like these are made we can see the effect on the representative sample of js output files.

Comment thread src/preamble.js Outdated
Comment thread emscripten.py Outdated
@kripken kripken merged commit acad9db into incoming Sep 17, 2019
@kripken kripken deleted the ref branch September 17, 2019 23:48
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
This moves that code from runtime addition of fields to the env, to compile-time addition and emitting of the final import object as a whole.

This has no significant functional change. It just makes the emitted code smaller and simpler. This also simplifies the overall model, and will allow further refactoring to simplify the dynamic linking code (which currently has its own computation of the wasm imports), leaving that for a later PR to keep this one as small as possible. This will also help with the code splitting experiment in the future.

This fixes the acorn parser passes to handle wasm import fields that are not just names, as now they can also be numbers or more complex expressions (the stuff we compute at compile time now may include such things).

The metadce tests get lots of updates, but there is no substance to them: the tests look at the initial wasm import object, before we added more fields at runtime. We don't add the fields at runtime anymore, so the tests can see those extra fields now, but they were always there, just the tests didn't notice them before. So there is no increase in the number of wasm imports, it's the same.

When testing this I also noticed running random tests was broken on fastcomp, we were just missing an if in runner.py, I added that here.
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 this pull request may close these issues.

2 participants