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

nvcc wrapper: Add -ccbin only if not already supplied #54

Merged
merged 9 commits into from
Dec 10, 2020
Merged

nvcc wrapper: Add -ccbin only if not already supplied #54

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
51bc9b6
Fix nvcc wrapper argument passing
mbargull Dec 7, 2020
1949ad6
Use exec for nvcc wrapper
mbargull Dec 7, 2020
71b3ec9
Avoid passing on -ccbin if already given
mbargull Dec 7, 2020
4ce54ab
MNT: Re-rendered with conda-build 3.20.5, conda-smithy 3.8.5, and con…
mbargull Dec 7, 2020
d230660
Bump build number
mbargull Dec 7, 2020
337557b
Restrict GCC versions for older CUDA
mbargull Dec 7, 2020
ed06c2e
Revert "Restrict GCC versions for older CUDA"
mbargull Dec 7, 2020
b5f40fe
Beautify wrapper script creation
mbargull Dec 7, 2020
95c76f3
Temporarily use GCC 7 for older CUDAs again
mbargull Dec 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .azure-pipelines/azure-pipelines-linux.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions .ci_support/linux_64_cuda_compiler_version10.0.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
c_compiler:
- gcc
c_compiler_version:
- '7'
- '9'
cdt_name:
- cos6
channel_sources:
Expand All @@ -15,9 +15,9 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '7'
- '9'
docker_image:
- condaforge/linux-anvil-cuda:10.0
- quay.io/condaforge/linux-anvil-cuda:10.0
target_platform:
- linux-64
zip_keys:
Expand Down
6 changes: 3 additions & 3 deletions .ci_support/linux_64_cuda_compiler_version10.1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
c_compiler:
- gcc
c_compiler_version:
- '7'
- '9'
cdt_name:
- cos6
channel_sources:
Expand All @@ -15,9 +15,9 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '7'
- '9'
docker_image:
- condaforge/linux-anvil-cuda:10.1
- quay.io/condaforge/linux-anvil-cuda:10.1
target_platform:
- linux-64
zip_keys:
Expand Down
6 changes: 3 additions & 3 deletions .ci_support/linux_64_cuda_compiler_version10.2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
c_compiler:
- gcc
c_compiler_version:
- '7'
- '9'
cdt_name:
- cos6
channel_sources:
Expand All @@ -15,9 +15,9 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '7'
- '9'
docker_image:
- condaforge/linux-anvil-cuda:10.2
- quay.io/condaforge/linux-anvil-cuda:10.2
target_platform:
- linux-64
zip_keys:
Expand Down
6 changes: 3 additions & 3 deletions .ci_support/linux_64_cuda_compiler_version11.0.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
c_compiler:
- gcc
c_compiler_version:
- '7'
- '9'
cdt_name:
- cos6
channel_sources:
Expand All @@ -15,9 +15,9 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '7'
- '9'
docker_image:
- condaforge/linux-anvil-cuda:11.0
- quay.io/condaforge/linux-anvil-cuda:11.0
target_platform:
- linux-64
zip_keys:
Expand Down
6 changes: 3 additions & 3 deletions .ci_support/linux_64_cuda_compiler_version9.2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
c_compiler:
- gcc
c_compiler_version:
- '7'
- '9'
cdt_name:
- cos6
channel_sources:
Expand All @@ -15,9 +15,9 @@ cuda_compiler_version:
cxx_compiler:
- gxx
cxx_compiler_version:
- '7'
- '9'
docker_image:
- condaforge/linux-anvil-cuda:9.2
- quay.io/condaforge/linux-anvil-cuda:9.2
target_platform:
- linux-64
zip_keys:
Expand Down
7 changes: 6 additions & 1 deletion recipe/install_nvcc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ EOF
mkdir -p "${PREFIX}/bin"
cat > "${PREFIX}/bin/nvcc" <<EOF
#!/bin/bash
"\${CUDA_HOME}/bin/nvcc" -ccbin "\${CXX}" \$@
for i ; do
case \$i in -ccbin)
exec "\${CUDA_HOME}/bin/nvcc" "\${@}"
esac
done
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to just loop through ${@} and see if -ccbin is there without the exec? Might simplify things a bit 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it make sense to just loop through ${@}

This is exactly what for i does ;)

without the exec?

The exec is just an independent improvement: If we have a launcher script, it makes sense to exec the called process so that our shell process can be stopped/replaced.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting would have expected that to look more like for i in "${@}". It might be clearer to write that (even if bash is doing some magic under-the-hood).

Copy link
Member Author

Choose a reason for hiding this comment

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

(even if bash is doing some magic under-the-hood).

It's standard POSIX shell: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_04_03

Interesting would have expected that to look more like for i in "${@}". It might be clearer to write that

I don't mind either way -- I can add a commit to change that :).

Copy link
Member

Choose a reason for hiding this comment

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

It would make it clearer I think. We already are cating into a script (my fault 😞). So it's already a bit confusing. The less confusing we can make it the better 🙂

exec "\${CUDA_HOME}/bin/nvcc" -ccbin "\${CXX}" "\${@}"
EOF
chmod +x "${PREFIX}/bin/nvcc"
2 changes: 1 addition & 1 deletion recipe/meta.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{% set name = "nvcc" %}
{% set number = 7 %}
{% set number = 8 %}

package:
name: "{{ name }}"
Expand Down