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

Improvements to JITLink, to ease transition #54841

Merged
merged 2 commits into from
Oct 5, 2024
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jun 18, 2024

Seeing what this will look like, since it has a number of features (delayed compilation, concurrent compilation) that are starting to become important, so it would be nice to switch to only supporting one common implmenetation of memory management.

Refs #50248

I am expecting llvm/llvm-project#63236 may cause problems, since we reconfigured some CI machines to hide that issue, but it is still likely relevant.

@giordano giordano added the compiler:llvm For issues that relate to LLVM label Jun 18, 2024
@vchuravy
Copy link
Member

vchuravy commented Jun 18, 2024

PPC got further than I expected.

Seems like we should try backporting:

@gbaraldi
Copy link
Member

Both are on LLVM 18 so I wonder if we should skip a release with 17, since the big changes that cause issues for people are already there (no opaque ptrs)

@giordano
Copy link
Contributor

I wonder if we should skip a release with 17

I don't think we're committed to have each major LLVM version in a Julia minor version: Julia v1.5 had LLVM 9, v1.6 had LLVM 11.

@vtjnash vtjnash marked this pull request as ready for review October 4, 2024 15:04
@vtjnash vtjnash requested a review from gbaraldi October 4, 2024 15:04
@vtjnash vtjnash changed the title Transition to JITLink Improvements to JITLink, to ease transition Oct 4, 2024
@vtjnash vtjnash requested a review from vchuravy October 4, 2024 15:06
if (!decls.specFunctionObject.empty())
NewDefs.push_back(decls.specFunctionObject);
}
auto Addrs = jl_ExecutionEngine->findSymbols(NewDefs);
Copy link
Member

Choose a reason for hiding this comment

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

This is the thing we discussed that makes the memory a little better right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this helps because it ensures it does the whole graph at once (behind the codegen lock). We can also later turn off the logic that tries to filter the graph, but this already should give the performance effect

Seeing what this will look like, since it has a number of features
(delayed compilation, concurrent compilation) that are starting to
become important, so it would be nice to switch to only supporting one
common implementation of memory management.

Refs #50248

I am expecting llvm/llvm-project#63236 may
cause some problems, since we reconfigured some CI machines to minimize
that issue, but it is still likely relevant.
@vtjnash vtjnash merged commit 0c8c20c into master Oct 5, 2024
7 checks passed
@vtjnash vtjnash deleted the jn/jitlink-always branch October 5, 2024 13:52
Zentrik added a commit to Zentrik/julia that referenced this pull request Oct 10, 2024
maleadt pushed a commit to Zentrik/julia that referenced this pull request Oct 11, 2024
Zentrik added a commit to Zentrik/julia that referenced this pull request Oct 12, 2024
Zentrik added a commit to Zentrik/julia that referenced this pull request Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:llvm For issues that relate to LLVM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants