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

Upstream merge #127

Merged
merged 752 commits into from
Sep 29, 2020
Merged

Upstream merge #127

merged 752 commits into from
Sep 29, 2020

Conversation

MatthewFluet
Copy link
Collaborator

Merge MLton/mlton upstream changes to master.

Currently, this is a work in progress. All of the merge conflicts have been "resolved" and both the runtime and the compiler proper compile without errors.

[I'm now noticing that the merge "restored" a number of files that were deleted by MaPLe but modified by MLton. Seems to have been a default behavior with magit; I'll figure out a way to fix that.]

It is getting more difficult to merge upstream changes, especially ones that touch the runtime system (no surprise), but also the build system (i.e., Makefiles). There are also unnecessary complications due to whitespace (e.g., due to MPL authors having a tendency to "fix" all the whitespace in a file when it is edited, which then results in merge conflicts when upstream makes edits near those whitespace fixes).

However, there is one upstream feature that is unclear how to incorporate into MaPLe, especially since MaPLe has chunkified the global heap (#93). MLton/mlton#357 (revising MLton/mlton#328) introduced a number of "static heaps" into the compilation. Essentially, many "global" objects can be fully evaluated at compile time and represented in the compiled program as statics. The "static" objects look like regular ML objects (with proper headers, etc.), but exist outside the MLton heap.

The question is how to handle this feature in the MaPLe fork.

One approach is to disable the compiler passes that lift objects to statics. For most objects, this will lead to behavior as is currently happening in MaPLe: most "global" objects get evaluated (and allocated) by the "initGlobals" function and assigned to a globalObjptr slot; they will be allocated in the hiearchical heap of the main thread. However, one exception to this are the WordXVector constants (i.e., strings). If they were not previously lifted to immutable statics, then they are collected at the RSSA to Machine translation and organized into an InitDynamic static heap that (in MLton) is memcpy-ed into the initial dynamic heap at program start. This would be harder to adjust to MaPLe, because we may need multiple chunks to hold this initial data. Also, there are references to these objects in the globalObjptr array emitted by the compiler; such references are updated by translateHeap in MLton, but would require a more sophisticated translation in MaPLe.

Another approach is to allow the lifting of objects to statics (or at least some of the different "kinds" of statics). This would require avoiding using HH operations (e.g., HM_getChunkOf) on such object pointers. We can detect if an object pointer references an object in a static heap by a series of range checks (e.g., static.start <= p && p < static.start + static.size). One could make it slightly faster by using a header bit to mark static objects. I'm not sure how many places would need to be changed to support this. We could certainly limit ourselves to the Immutable and Mutable static heaps, and not allow the Root or InitDynamic static heaps. The Immutable static heap is self describing (and is RO data in the executable). The Mutable static heap is comprised of mutable objects without mutable objptr fields (and is RW in the executable). The Root static heap is comprised of mutable objects with mutable objptr fields; such objects need to be treated as roots for garbage collection. (We actually don't use the Root static heap in MLton, because card marking would fail when trying to mark the card that corresponds to an object in the Root static heap. For a similar reason, I would avoid using the Root static heap in MaPLe.)

Yet another approach is to allow the Immutable, Mutable, and Root static heaps, but emit them as chunk objects, so that things like HM_getChunkOf works. Of course, then it may not be possible for the Immutable static heap (static chunks) to be RO data, since there may be operations that mutate the Chunk Metadata. There is also the overhead of maintaining code to properly produce chunk metadata in the compiler proper (and handling things like the linked lists of chunks).

We could attempt to "back out" the static heap changes.

Finally, we could simply close the PR without merging and forgoe future upstream merges.

MatthewFluet and others added 30 commits January 17, 2020 15:54
The debug runtimes are unused on TravisCI; this avoids unnecessary
build time.
When position independent code is generated by the C compiler by
default, then the `%.a` libraries can be used as `%-pic.a` libraries.
This avoids redundant compiles of the runtime system.
Updates to `make runtime`

Highlights:

* Use proper Makefile rules for `/runtime/gen/sizes`, `/runtime/gen/ml-types.h`,
  `/runtime/gen/c-types.{h,sml}`, and `/runtime/gen/basis-ffi.{h,sml}`, making
  the generator executables a dependency, updating the generator programs to
  emit each file separately (according to program argument), and eliminating the
  use of `.stamp` files (45041d9, 36494b0, ac1651a, 6a37cc3).

* Avoid building `runtime/gen/basis-ffi.{h,sml}` with clean sources (76da64c).

* Use automatic dependencies via `cc -MM` and `.d`-files (eeee00f).

* Add and use `WITH_{DBG,PIC}_RUNTIME` in `runtime/Makefile` (18c6f0c).

* Build with `WITH_DBG_RUNTIME=false` in `./bin/travis-ci` (84d4861).

* Copy `%-pic.a` from `%.a` when PIC enabled by default (a864e2c).

The last two, in particular, allow TravisCI to only build one variant of the
runtime, rather than 3.

The goal had been to bring some order to `runtime/Makefile` and potentially
better understand how to handle non-PI vs PIC vs PIE (see MLton/mlton#191).
The `<name> = _symconst : <ty>` form becomes

    const <cty> <name>;

in `basis-ffi.h` and becomes

    MPLLang#1 (_symbol "<name>" : (unit -> <ty>) * (<ty> -> unit);) ()

in `basis-ffi.sml`.
This commit begins migrating the `make constants` functionality from
the compiler proper to the runtime.  Almost all `_const`-s in the
Basis Library are derived from `basis-ffi.def` and can be generated at
the time that the runtime is built.  The `make constants` step is
awkward and complicates porting.
This will (eventually) replace `LookupConstant`.
Refactor gen and use of `constants` file

Migrate the `make constants` functionality from the compiler proper to
the runtime. All `_const`-s in the Basis Library are derived `from
basis-ffi.def` and are generated at the time that the runtime is
built. Eliminate the `make constants` step, which is awkward and
complicates porting.
Support parallel builds (i.e., `make -j`)

Simply perform separate `$(MAKE)` commands for sequential steps in
`Makefile`; in particular, for the `all` rule, perform `$(MAKE) dirs;
$(MAKE) runtime` rather than `$(MAKE) dirs runtime` and similarly for
other places where recursive `$(MAKE)` was invoked with multiple
(`.PHONY`) targets.

This mainly supports platforms/packagers that use a parallel make by
default; it does not obtain significant build speedups.  The main
(time consuming) self-compile is necessarily sequential.  On the other
hand, the runtime system can be built in parallel.

Closes MLton/mlton#132
Closes MLton/mlton#348
MatthewFluet and others added 7 commits August 30, 2020 12:38
Guide updates

* Update `SMLNJLibrary` and `MLRISCLibrary` pages in guide
* Add contributed MinGW binary packages
Fix links to MinGW packages on SourceForge
During a DFS marking, a weak object should only be linked if its
objptr field is a valid object pointer (and not `BOGUS_OBJPTR`).

This bug was introduced by commit ec2a3af, which changed
`is{Objptr,Pointer}` predicates to `is{Objptr,Pointer}InHeap`
predicates.  This was motivated by the introduction of static heaps,
because objects in the static heaps are not traced; in particular,
objects in the immutable static heap are in read-only memory, and
therefore cannot be marked and unmarked.

However, the semantics of these predicates are subtly different.  The
`is{Objptr,Pointer}InHeap` return `TRUE` when the argument is a
non-pointer (i.e., when `is{Objptr,Pointer}` returns `FALSE`).

Commit 2d46cc1 fixed many of the `isPointerInHeap(p)` predicates to
`isPointer(p) and isPointerInHeap(p)`, but missed the `isObjptrInHeap`
that guards the linking of a weak object.
Fix bug in handling of weak objects during mark-compact GC
@shwestrick
Copy link
Collaborator

Wow, thank you for putting so much effort into this! We'll try to be more careful with the whitespace changes--sorry about that. Do you have any thoughts on how else we could make these merges simpler, going forward?

As for the static heaps, I have some questions/thoughts.

Suppose we disabled the relevant compiler passes. Then it sounds like the only thing is figuring out how to handle the InitDynamic heap at program startup. Would this be more difficult than the change we made for chunkifying the global heap? For example, in current MPL, we do a pass over static data with initVectors at program startup. Has the InitDynamic heap replaced this?

I like the idea of allowing static objects and special casing static pointers (e.g. by checking static.start <= p && p < static.start + static.size as you suggest). Here's a thought along these lines: we could change chunks slightly to allow the chunk header to be stored anywhere, not necessarily next-to/inside the chunk itself. This way, the chunk headers for static objects could be stored elsewhere and could still be mutable even when the "contents" of the chunk are not. I don't think it would be a huge change to the runtime. Let me think about it...

@MatthewFluet
Copy link
Collaborator Author

After some more thought, I think that you are correct that handling a Dynamic static heap is the path of least resistance. Yes, the Dynamic static heap has replaced the initVectors mechanism. My initial concern was that a general Dynamic heap can have more than just WordXVector constants; it could have arbitrary objects with fields that are references to other objects in the Dynamic heap. In MLton, translateHeap works fine to update all of the references within the copied heap (it does a foreachObjptrInRange that performs a shift on each objptr. If the Dynamic heap were split into multiple chunks, then such translations would be more complicated. However, it should be the case that if we disable all of the collectStatics.* passes, then the Dynamic heap should contain only WordXVector constants, which don't have any references to other objects. Now we can follow the model of the chunkified-globals handling of initVectors --- walk the Dynamic heap to find the set of objects that fit into one chunk, memcpy that segment, scan over the globalObjptr to update any references that have been copied (this can be a simple shift of any pointer in the range of the memcpy'd segment from the starting address of the memcpy'd segment to the starting address of the chunk), and then continue walking the Dynamic heap to find the next segment that fits into a chunk. Because we only need to scan globalObjptr, this should be somewhat simpler.

Later one, one could consider how to better support the other kinds of static heaps.

@MatthewFluet
Copy link
Collaborator Author

(Re)Merge MLton/mlton upstream changes to master, taking care to not restore files deleted by MaPLe.

MatthewFluet added a commit to MatthewFluet/mpl that referenced this pull request Sep 24, 2020
MLton/mlton#357 (revising MLton/mlton#328) introduced a number of "static heaps"
into the compilation.  Essentially, many "global" objects can be fully evaluated
at compile time and represented in the compiled program as statics.  The
"static" objects look like regular ML objects (with proper headers, etc.), but
exist outside the MLton heap.

This is a "path of least resistance" commit for MaPLe.  The
`collectStatics.Globals` and `collectStatics.RealConsts` passes are disabled,
but the `collectStatics.WordXVectorConsts` pass is enabled.  In the `backend`
pass (translation of RSSA to Machine), all RSSA statics are forced to the
`Dynamic` static heap (and assigned a corresponding global objptr slot);
similarly, any remaining `WordXVector` constants are forced to the `Dynamic`
static heap.  At program startup, the `Dynamic` static heap is copied into the
root hierarchical heap.  (This is slightly more complicated than the copy of the
`Dynamic` static heap into the initial heap in MLton, because in MaPLe the
`Dynamic` static heap may need to be split across multiple chunks.)

See MPLLang#127 for more discussion.
MatthewFluet added a commit to MatthewFluet/mpl that referenced this pull request Sep 25, 2020
MLton/mlton#357 (revising MLton/mlton#328) introduced a number of "static heaps"
into the compilation.  Essentially, many "global" objects can be fully evaluated
at compile time and represented in the compiled program as statics.  The
"static" objects look like regular ML objects (with proper headers, etc.), but
exist outside the MLton heap.

This is a "path of least resistance" commit for MaPLe.  The
`collectStatics.Globals` and `collectStatics.RealConsts` passes are disabled,
but the `collectStatics.WordXVectorConsts` pass is enabled.  In the `backend`
pass (translation of RSSA to Machine), all RSSA statics are forced to the
`Dynamic` static heap (and assigned a corresponding global objptr slot);
similarly, any remaining `WordXVector` constants are forced to the `Dynamic`
static heap.  At program startup, the `Dynamic` static heap is copied into the
root hierarchical heap.  (This is slightly more complicated than the copy of the
`Dynamic` static heap into the initial heap in MLton, because in MaPLe the
`Dynamic` static heap may need to be split across multiple chunks.)

See MPLLang#127 for more discussion.
MatthewFluet and others added 7 commits September 25, 2020 10:34
Motivated by downstream changes in MaPLe fork that wants to omit building tools.
In addition to resolving merge conflicts:

* Use `GC_foreachObjptrClosure` from upstream,
  rather than the "separated" closure from MPLLang/mpl.
  Also, introduce `GC_objptrPredicateClosure` to
  replace the "separated" closure from MPLLang/mpl.

* Minimize diff with upstream for `Makefile`
  Prefer `ifneq (OMIT_FEAT, true) ... endif` over `# ...`.
  This will reduce conflicts when merging upstream changes.
MLton/mlton#357 (revising MLton/mlton#328) introduced a number of "static heaps"
into the compilation.  Essentially, many "global" objects can be fully evaluated
at compile time and represented in the compiled program as statics.  The
"static" objects look like regular ML objects (with proper headers, etc.), but
exist outside the MLton heap.

This is a "path of least resistance" commit for MaPLe.  The
`collectStatics.Globals` and `collectStatics.RealConsts` passes are disabled,
but the `collectStatics.WordXVectorConsts` pass is enabled.  In the `backend`
pass (translation of RSSA to Machine), all RSSA statics are forced to the
`Dynamic` static heap (and assigned a corresponding global objptr slot);
similarly, any remaining `WordXVector` constants are forced to the `Dynamic`
static heap.  At program startup, the `Dynamic` static heap is copied into the
root hierarchical heap.  (This is slightly more complicated than the copy of the
`Dynamic` static heap into the initial heap in MLton, because in MaPLe the
`Dynamic` static heap may need to be split across multiple chunks.)

See MPLLang#127 for more discussion.
@MatthewFluet MatthewFluet marked this pull request as ready for review September 25, 2020 16:35
@MatthewFluet
Copy link
Collaborator Author

O.k., I was able to implement a simple initDynHeap function based on initVectors that copies the Dynamic static heap in to the root hierarchical heap. It only supports WordXVector constants that are collected by the collectStatics.WordXVectorConsts pass, because such objects have no objptr fields that might reference other objects in the Dynamic static heap. The other collectStatics.Globals and collectStatics.RealConsts passes are disabled.

I was able to follow all of the MPLang/mpl-tutorial with this build, so it must be working!

@MatthewFluet
Copy link
Collaborator Author

I think this is ready for merge. Or, at the very least, I don't have any more plans for the merge. At some later time, we could consider richer support for static objects, but this at least merges the upstream changes while preserving MaPLe's current practice (of initializing the root hierarchical heap with string data at program initialization).

@shwestrick
Copy link
Collaborator

I was just finishing up some tests on various benchmarks :) It seems to be working great -- thanks Matthew!

@shwestrick shwestrick merged commit b6eed29 into MPLLang:master Sep 29, 2020
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.

5 participants