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

Move globals out of the MVP #344

Merged
merged 3 commits into from
Sep 16, 2015
Merged

Move globals out of the MVP #344

merged 3 commits into from
Sep 16, 2015

Conversation

lukewagner
Copy link
Member

Based on a lot of discussion in #154, I'm proposing we remove globals from the MVP. This PR also discusses the different things we want post-MVP: immutable global pointers for dynamic linking, mutable thread-local variables for threads, and a new future feature for real mutable global variables that captures the use cases.

@kripken
Copy link
Member

kripken commented Sep 11, 2015

lgtm.

linear memory. This would allow more aggressive compiler optimizations (due to
better alias information). If globals are additionally allowed array types,
significant portions of memory could be moved out of linear memory which could
reduce fragmentation issues. Langauges like FORTRAN which limit aliasing would be
Copy link
Member

Choose a reason for hiding this comment

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

Langauge fail.

Copy link
Member

Choose a reason for hiding this comment

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

Nowadays it's Fortran. No more yelling.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@jfbastien
Copy link
Member

lgtm, but it would be good to leave this open until early next week, so @titzer and others who are supposed to be on vacation can take a look :-)

@kg
Copy link
Contributor

kg commented Sep 14, 2015

If we do this we should consider adding a way to explicitly set a symbol's numeric value. That will let us assign the heap locations of global state names in our test cases (and make it possible to round-trip them in a decompile/compile loop).

@lukewagner
Copy link
Member Author

At first I was confused by your use of "symbol" (interpreting "symbol" in the context of the hunk added to DynamicLinking.md by this PR) since immutable global pointers are post-MVP. But I think you're talking about "debugging symbols", as in the separate, optional module section which is purely added to assist projection of binary to text. In that case, I totally agree; I just added a section to TextFormat.md describing this interaction.

@jfbastien
Copy link
Member

We (@ncbray, @flagxor, @lukewagner, @kripken, @sunfishcode) had a chat about this offline, and agree that this patch can indeed land as-is. We had concerns that for MVP we may want a more restricted form of globals to address specific performance concerns, and Mozilla folks expressed their willingness to revisit new-MVP-globals if/when data shows that we need to revisit.

jfbastien added a commit that referenced this pull request Sep 16, 2015
@jfbastien jfbastien merged commit 7b1477e into master Sep 16, 2015
@jfbastien jfbastien deleted the rm-globals branch September 16, 2015 22:11
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.

4 participants