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

Support cross-compilation for use with Nerves #1543

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

fhunleth
Copy link
Contributor

@fhunleth fhunleth commented Oct 9, 2024

This moves the Makefile code that queries the system type and CUDA
availability under a guard for whether things are being cross-compiled.
If cross-compiled, then the settings are hardcoded. This should work in
non-Nerves compilation cases too, but those weren't tested.

This moves the Makefile code that queries the system type and CUDA
availability under a guard for whether things are being cross-compiled.
If cross-compiled, then the settings are hardcoded. This should work in
non-Nerves compilation cases too, but those weren't tested.
Copy link
Collaborator

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Fine by me, thank you! But @polvalente should merge it :)

@fhunleth
Copy link
Contributor Author

fhunleth commented Oct 9, 2024

Thanks!

@polvalente In case you're curious, @lawik has been working with the Whisper model and we've been collaborating on nerves-livebook/nerves_livebook#559. Note that Whisper doesn't work on that PR due to a lack of ffmpeg, but much simpler use seems to be working.

Comment on lines +57 to +62
else
# Determine settings for cross-compiled builds like for Nerves
UNAME_S = Linux
NVCC := $(CXX)
NVCCFLAGS = $(CFLAGS)
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe UNAME_S should be given as an optional env flag. Linux is probably always the target, but I don't think we can assert this for sure. Maybe a different name should be used for the env var as it's now gonna be (optionally) exposed.

I also wonder if we should allow NVCC to be provided via the toolchain for systems like the nvidia jetson.

Suggested change
else
# Determine settings for cross-compiled builds like for Nerves
UNAME_S = Linux
NVCC := $(CXX)
NVCCFLAGS = $(CFLAGS)
endif
else
# Determine settings for cross-compiled builds like for Nerves
UNAME_S ?= Linux
NVCC := $(CXX)
NVCCFLAGS = $(CFLAGS)
endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, given that this is only used for checking if it's mac or not, we can merge as is. Crosscompiling for mac is highly unlikely IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only cross-compilation I've run into that hasn't been Linux is for RTOSs and the "Linux" options seem to work. It might be easier to address when it comes up.

Thanks for merging!

@polvalente polvalente merged commit 3a92566 into elixir-nx:main Oct 10, 2024
7 of 8 checks passed
@fhunleth fhunleth deleted the support-cross-compilation branch October 10, 2024 01:10
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.

3 participants