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.

Sign up for GitHub

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

Jump to bottom

Add support for LTO in Numba linker #11

Merged
merged 4 commits into from
Nov 20, 2023
Merged

Add support for LTO in Numba linker #11

merged 4 commits into from
Nov 20, 2023

Conversation

gmarkall
Copy link
Contributor

@gmarkall gmarkall commented Nov 2, 2023

Support for enabling LTO via nvjitlink is added - it is enabled by passing the lto=True kwarg. I also made a couple of other related changes:

  • The minimum CUDA toolkit version is bumped to 12.3 - there's no point building against anything older, especially for LTO use cases.
  • I noticed there were no tests of the patch, so I ported those from CubinLinker. Note that although CubinLinker is not open-source, the source of its tests is included in the distribution, so this addition only brings things that were already public into pynvjitlink.

cc @leopoldcambier to ensure this meets your requirements for LTO use cases.

@gmarkall
Add support for LTO and linking object files
28d9909 
The `lto` kwarg provides a convenience option for enabling LTO. Other
flags might also be required by users; rather than attempting to
maintain a mapping of all flags, we enable arbitrary additional flags to
be passed in for now.
@gmarkall
Add patch tests based on CubinLinker patch tests
1436aa8 
This also requires the `PatchedLinker` to store its options for
inspection (which feels like it should be generally handy for a user to
be able to check anyway).
@gmarkall
Bump CUDA toolkit version requirement to 12.3
e49ad49
@gmarkall gmarkall requested a review from a team as a code owner November 2, 2023 16:19
wence-
wence- approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me, I think.

vyasr
vyasr approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Seems fine especially since it's largely just forwarding along arguments.

@gmarkall
Copy link
Contributor Author

I just merged main so that CI would run on this PR now that #10 is merged.

@brandon-b-miller brandon-b-miller merged commit f5affdb into rapidsai:main Nov 20, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandon-b-miller brandon-b-miller brandon-b-miller approved these changes

@vyasr vyasr vyasr approved these changes

@wence- wence- wence- approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gmarkall @wence- @vyasr @brandon-b-miller