Conditionally compile GC source files. Test stock build in CI. #73
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
qinsoon
commented
Nov 26, 2024
| @@ -270,7 +270,10 @@ JL_DLLEXPORT void jl_genericmemory_copyto(jl_genericmemory_t *dest, char* destda | |||
| _Atomic(void*) * dest_p = (_Atomic(void*)*)destdata; | |||
| _Atomic(void*) * src_p = (_Atomic(void*)*)srcdata; | |||
| jl_value_t *owner = jl_genericmemory_owner(dest); | |||
| jl_gc_wb(owner, NULL); // FIXME: needs to be added here since the check below doesn't apply to MMTk | |||
| // FIXME: The following should be a write barrier impl provided by the GC. | |||
There was a problem hiding this comment.
We should report this case to the Julia team. @udesou
There was a problem hiding this comment.
Good point. I added that comment there to keep track of it, so it would show up in a PR that includes sticky immix since IIRC we need that barrier for it to work.
There was a problem hiding this comment.
Yeah. The comment is helpful.
udesou
approved these changes
Nov 26, 2024
There was a problem hiding this comment.
LGTM. Since you're doing that, could you also remove the #ifdefs from the .h files such as gc-tls-mmtk.h, gc-tls.h, gc-stock.h and gc-mmtk.h. I'm not sure we need to change the Makefile to conditionally consider them as well, but they're imported conditionally already in julia_threads.h.
qinsoon
added a commit
to mmtk/mmtk-julia
that referenced
this pull request
Nov 27, 2024
Update to mmtk/julia#73 -- conditionally include source files based on the GC implementation.
udesou
pushed a commit
that referenced
this pull request
Nov 29, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
udesou
pushed a commit
that referenced
this pull request
Dec 2, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
udesou
pushed a commit
that referenced
this pull request
Dec 3, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
udesou
pushed a commit
that referenced
this pull request
Dec 9, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
udesou
pushed a commit
that referenced
this pull request
Dec 9, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
udesou
pushed a commit
that referenced
this pull request
Dec 12, 2024
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor `#ifdef` for related files. This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the build script to only include source files for the GC implementation that is selected. The PR also removes unnecessary preprocessor
#ifdeffor related files.This PR adds a CI job to build with the stock GC. This helps us to make sure that the stock GC can build.