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

Remove STATIC* JS vars and STATIC_BUMP setting #12176

Merged
merged 4 commits into from
Sep 12, 2020
Merged

Remove STATIC* JS vars and STATIC_BUMP setting #12176

merged 4 commits into from
Sep 12, 2020

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 11, 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).

In principle since all that layout was done by lld, we could just receive it from
there instead of computing it in emscripten.py. Perhaps finalize should return
the value of __heap_base and other relevant globals? Is there a list of all
of them?

See #11860

@kripken kripken requested a review from sbc100 September 11, 2020 22:35
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Great!!

@dschuff
Copy link
Member

dschuff commented Sep 11, 2020

This is nice, but I also like the idea of getting more from the linker and tracking less in the metadata. Could we use a link map for this purpose? (I know we had a discussion about link maps sometime recently but I can't find it atm).

@sbc100
Copy link
Collaborator

sbc100 commented Sep 11, 2020

Here is the map file PR: https://reviews.llvm.org/D77187

I agree hopefully there is a lot more cleanup to do here but this is an important step to get landed.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 11, 2020

I agree, using the map file is an interesting idea because it allows us get to get information about the wasm without needing extra exports.

@kripken
Copy link
Member Author

kripken commented Sep 11, 2020

Sounds like the link map will let us read all the layout info from there? That could be a nice further simplification to remove all the Memory() computations in emscripten.py.

@kripken kripken merged commit e8023f1 into master Sep 12, 2020
@kripken kripken deleted the reduce-STATIC branch September 12, 2020 00:00
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.

3 participants