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

Fastcomp cleanup post removal #11860

Closed
5 of 9 tasks
sbc100 opened this issue Aug 11, 2020 · 5 comments
Closed
5 of 9 tasks

Fastcomp cleanup post removal #11860

sbc100 opened this issue Aug 11, 2020 · 5 comments

Comments

@sbc100
Copy link
Collaborator

sbc100 commented Aug 11, 2020

As a followup to #11319 we have a lot of potential cleanup of emscripten to do now that fastcomp is no more.

One thing to consider: do we want to make backporting/cherrying picking easy(er) for while). Should we hold off large refactors for a while for this reason? It seem like there will be enough pure-deletion PRs to keep us going for while :)

List of cleanup work:

  • Remove asmjs validation (Remove asmjs validation #11551).
  • Remove JS in src/ inside !WASM_BACKEND
  • Remove check_vanilla and is_vanilla cache file, @sbc
  • Remove all Python driver code inside not WASM_BACKEND
  • Remove WASM_BACKEND completely.
  • Remove fastcomp-only settings in settings.js.
  • Remove dual/split cache (cache/asmjs vs cache/wasm)
  • Remove fastcomp backend code in emscripten.py.
  • Remove WASM? (even with -s WASM=0 we set WASM to 1 and set WASM2JS, in verify_settings. so WASM == 1 all the time in tests etc.)

Cleanup PRs should mention "See #11860" (this PR).

@kripken
Copy link
Member

kripken commented Aug 11, 2020

I agree deletion PRs are a good starting point. That's low-risk, and enables more stuff later.

I don't think we should try to make backporting easier, as there seems little benefit. (If backporting is important to someone, would be good to hear about that, and how we can work together on it.)

I think we should coordinate those deletion PRs in this issue, to avoid stepping on each others' toes and getting lots of merge conflicts. For example: I can start with the JS code src/* - ?

@sbc100
Copy link
Collaborator Author

sbc100 commented Aug 11, 2020

Sounds good. Why don't we all work on the list above and put your name on a list item if you are going to start work on it? I am personally going to try to avoid the temptation to just do all of them right away :) (Mostly since I think it could suck all my time for the next month!)

kripken added a commit that referenced this issue Aug 11, 2020
After removing it, things are always in DOUBLE_MODE == 1
basically (that is, doubles are normal doubles, no asm.js weirdness).

See #11860
Akaricchi added a commit to Akaricchi/emscripten that referenced this issue Aug 12, 2020
sbc100 pushed a commit that referenced this issue Aug 12, 2020
kripken added a commit that referenced this issue Aug 12, 2020
This has been a no-op on wasm, and this PR keeps it that way (we do
duplicate function elimination in binaryen automatically).

See #11860
kripken added a commit that referenced this issue Aug 12, 2020
SEPARATE_ASM was always just an asm.js feature, and never
worked in upstream.
kripken added a commit that referenced this issue Aug 12, 2020
sbc100 added a commit that referenced this issue Sep 8, 2020
This is no longer in use since fastcomp removal.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
These are no longer used or usefull since fastcomp
removal.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
Also remove functions and modules that were only used
by this code.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
Also remove functions and modules that were only used
by this code.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
This was only used in fastcomp builds.  AFAICT the llvm
backend uses binaryen passes to do this stuff.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
Also remove functions and modules that were only used
by this code.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
These are no longer used or usefull since fastcomp
removal.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
This was only used in fastcomp builds.  AFAICT the llvm
backend uses binaryen passes to do this stuff.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
These are no longer used or usefull since fastcomp
removal.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
These are no longer used or useful since fastcomp
removal.

See: #11860
sbc100 added a commit that referenced this issue Sep 10, 2020
sbc100 added a commit that referenced this issue Sep 10, 2020
kripken added a commit that referenced this issue Sep 12, 2020
These used to make sense when we had static allocations in JS. We'd use the
"bump" to track the total size, which was increased by the JS compiler, and at
runtime we'd have STATIC_BASE and STATICTOP. Now that we disallow static
allocations from the JS compiler, we don't need them.

Note that we do still track "staticBump" in the metadata from finalize. That is
the total size of static memory from lld, which never changes. We use it to
compute where the stack begins (which determines the rest of memory layout).
As a followup we may be able to use a link map or other mechanism to just
read that info instead of computing it in emscripten.py

See #11860
kripken added a commit that referenced this issue Oct 5, 2020
This removes some TODOs from wasm2js where we thought about
running more of the old passes. I think that's not a good idea - they
will not run as our wasm2js output gets less and less like asm.js
anyhow. We'll need to write new acorn passes there, if we want to.

This removes safeHeap which we should be running (see #12410 (comment)),
but as it would be the only thing we use the js optimizer for from emcc.py,
and that isn't a recent regression, remove that too and I'll rewrite it for
acorn.

See #11860
@stale
Copy link

stale bot commented Aug 21, 2021

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Aug 21, 2021
@stale stale bot closed this as completed Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants