-
Notifications
You must be signed in to change notification settings - Fork 23
Add --target-cuda argument for selecting CUDA architecture
#2478
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
Conversation
|
View rendered docs @ https://intelpython.github.io/dpnp/index.html |
| if ("x${DPNP_SYCL_TARGETS}" STREQUAL "x") | ||
| if(DPNP_TARGET_CUDA) | ||
| set(_dpnp_sycl_targets "nvptx64-nvidia-cuda,spir64-unknown-unknown") | ||
| if (DPNP_TARGET_CUDA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not OFF by default now. Should this be updated?
| if (DPNP_TARGET_CUDA) | |
| if (NOT "x${DPNP_TARGET_CUDA}" STREQUAL "x") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The empty string is False for this check if (DPNP_TARGET_CUDA)
I added this check in case when DPNP_TARGET_CUDA is passed as 0, OFF, NO, FALSE, N via cmake-opts argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case when
DPNP_TARGET_CUDAis passed as0, OFF, NO, FALSE, Nviacmake-optsargument
That is not the case when DPNP_TARGET_CUDA passed as an empty string. So it's still unclear for me.
Per my understanding the string can't be empty due to the check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right that --target-cuda= is checked in build_locally.py.
But if someone bypasses it via --cmake-opts="-DDPNP_TARGET_CUDA=" the empty string is still evaluated as FALSE in if(DPNP_TARGET_CUDA). Thus this condition safely handles both cases.
Using if (NOT "x${DPNP_TARGET_CUDA}" STREQUAL "x") would only check for non-empty strings but still treat values like OFF or 0 as TRUE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it sounds reasonable.
But what is about similar flag for AMD build? Why don't we check the same there then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antonwolfy it will be updated in the next PR.
Thank you
|
Array API standard conformance tests for dpnp=0.19.0dev0=py312h509198e_23 ran successfully. |
antonwolfy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more comments from me.
Thank you @vlad-perevezentsev
This PR suggests adding `--target-cuda` argument to `scripts/build_locally.py` allowing to enable CUDA support and optionally specify the target architecture (e.g. `sm_80`). If no architecture is specified, `sm_50` is used by default. ```bash $ python scripts/build_locally.py --target-cuda # or $ python scripts/build_locally.py --target-cuda=<arch> ``` 32e1b4e
This PR suggests adding
--target-cudaargument toscripts/build_locally.pyallowing to enable CUDA support and optionally specify the target architecture (e.g.sm_80).If no architecture is specified,
sm_50is used by default.The specified architecture is used to construct a SYCL alias target (e.g.
nvidia_gpu_sm_80) and passed via-fsycl-targetsoption, following OneAPI for NVIDIA GPUs