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

Cross-compile linux_aarch64 & linux_ppc64le #58

Merged
merged 13 commits into from
Dec 19, 2023

Conversation

conda-forge-admin
Copy link
Contributor

@conda-forge-admin conda-forge-admin commented Dec 19, 2023

Enables cross-compilation for linux_aarch64 & linux_ppc64le builds (as opposed to emulation, which was used before). This significantly improves the build times of those platforms and should simplify maintenance here.

Was able to do a bit of cleanup in the process thanks to other fixes that have been made in the interim.

Did need to add one workaround for Windows. Though hopefully this can be dropped in the 12.2 upgrade with an upstream fix ( conda-forge/cuda-nvcc-impl-feedstock#4 )

Fixes #34

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@jakirkham jakirkham marked this pull request as draft December 19, 2023 03:14
@jakirkham
Copy link
Member

@conda-forge-admin , please re-render

jakirkham

This comment was marked as off-topic.

@jakirkham jakirkham changed the title MNT: rerender Cross-compile linux_aarch64 & linux_ppc64le Dec 19, 2023
Borrows the pinning of `cuda-version` from `host`.
Place Python 3.12 `skip` by itself. It is something we plan to drop
once we are able.

Also place CUDA `skip` by itself. This is something we need to hold onto
long term.

Drop macOS `skip` as it already falls out of the CUDA skip. So is
redundant.
Instead of having the CUDA version defined to build against in various
places, consolidate down to just the `version` used by `cuda-python` at
the beginning of the recipe. The propagate this value into how the
`skip`s are configured to select the relevant CUDA version. Also
leverage this in how `cuda-version` is constrained in the `requirements`
during the build phase. This should simplify the upgrade process of
`cuda-python` for new versions (including major CUDA version bumps).
@jakirkham
Copy link
Member

@conda-forge-admin , please re-render

Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/cuda-python-feedstock/actions/runs/7257034521.

@jakirkham
Copy link
Member

Seeing errors like this on CI:

  Error compiling Cython file:
  ------------------------------------------------------------
  ...
  cdef cudaError_t _cudaProfilerStart() nogil except ?cudaErrorCallRequiresNewerDriver:
      cdef cudaError_t err
      err = m_global.lazyInitContextState()
      if err != cudaSuccess:
          return err
      err = <cudaError_t>ccuda._cuProfilerStart()
           ^
  ------------------------------------------------------------

  cuda/_lib/ccudart/ccudart.pyx:3625:10: Casting temporary Python object to non-numeric non-Python type

Think these are masking the real issue, which is that cuda-python is having trouble finding cuda-profiler-api. This comes back to the fact that we encountered some issues with splayed layouts in the cuda-python recipe before ( #33 (comment) ) ( NVIDIA/cuda-python#46 )

We worked around this issue previously by placing some dependencies (like cuda-profiler-api) in requirements/build. However those packages will be installed to match the build_platform (not the target_platform), which means their targets contents will be in a directory determined by build_platform (and not target_platform where we search for headers and libraries). Hence this is causing headers for dependencies (like cuda-profiler-api) not to be found, which leads to opaque errors like those above (as Cython assumes these are Python objects as it doesn't see the C APIs as available)

Previously we pointed `CUDA_HOME` to `BUILD_PREFIX`. Though ideally we
would point to `PREFIX` as that is where headers and libraries for the
intended `target_platform` would be. So make this change. This may
require more changes given `cuda-python` does not support splayed
layouts.
This reverts commit 5ffc9e5.

It looks like `LIBRARY_PREFIX` is not defined when `script_env`s are
set. Given this, revert to `PREFIX`, which is defined when setting
`script_env`s.
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Looks like a lot of the previous issues on Linux that were seen here got cleaned up. Am guessing this came out of changes to how headers and libraries are searched for ( conda-forge/cuda-nvcc-feedstock#16 )

Windows ran into issues finding crt headers though. It doesn't have the same setup as Linux though. So we lack some of the same flexibility in setting flags. This should go away once crt headers can be added cleanly as a dependency in CUDA 12.2. For now added a simple workaround for Windows that with more details below

script:
- set "INCLUDE=%INCLUDE%;%BUILD_PREFIX%\Library\include" # [win64]
Copy link
Member

Choose a reason for hiding this comment

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

On Windows, the builds ran into an issue finding the crt headers on CI:

 building 'cuda._lib.ccudart.ccudart' extension
  creating build\temp.win-amd64-cpython-311\Release\cuda\_lib\ccudart
  "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\bin\HostX64\x64\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -I.\cuda -ID:\bld\cuda-python_1702961331834\_h_env -ID:\bld\cuda-python_1702961331834\_h_env\Library/include -ID:\bld\cuda-python_1702961331834\_h_env\include -ID:\bld\cuda-python_1702961331834\_h_env\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.29.30133\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um" -ID:\bld\cuda-python_1702961331834\_h_env\Library\include -ID:\bld\cuda-python_1702961331834\_h_env\Library\include -ID:\bld\cuda-python_1702961331834\_h_env\Library\include\targets\x64 /d1trimfile:D:\bld\cuda-python_1702961331834\work /EHsc /Tpcuda/_lib/ccudart\ccudart.cpp /Fobuild\temp.win-amd64-cpython-311\Release\cuda/_lib/ccudart\ccudart.obj
  ccudart.cpp
  D:\bld\cuda-python_1702961331834\_h_env\Library/include\vector_types.h(65): fatal error C1083: Cannot open include file: 'crt/host_defines.h': No such file or directory
  error: command 'C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.29.30133\\bin\\HostX64\\x64\\cl.exe' failed with exit code 2
  error: subprocess-exited-with-error

The crt headers are part of the compiler currently. This will be broken out as their own package in CUDA 12.2 ( conda-forge/cuda-nvcc-impl-feedstock#4 )

For now this adds the NVCC includes to the end of INCLUDE so they are searched as well (after exhausting all other search paths)

This fixes the Windows build issue with crt headers

Copy link
Member

Choose a reason for hiding this comment

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

Tracking this in issue: #74

Copy link
Member

Choose a reason for hiding this comment

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

Attempting to address this with PR: #75

@jakirkham jakirkham marked this pull request as ready for review December 19, 2023 06:31
@jakirkham
Copy link
Member

@conda-forge-admin , please re-render

@jakirkham jakirkham mentioned this pull request Dec 19, 2023
3 tasks
Copy link
Contributor

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/cuda-python-feedstock/actions/runs/7258263787.

@jakirkham
Copy link
Member

Thanks all! 🙏

Going to go ahead and merge this so we can use cross-compilation for the upgrades. Hope that is ok 🙂

@jakirkham jakirkham merged commit 16af942 into conda-forge:main Dec 19, 2023
18 checks passed
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.

Use cross-compilation instead
4 participants