Skip to content

[CUDA] Compilation for NVPTX target doesn't support -fsycl-device-code-split #1560

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
Closed
bader opened this issue Apr 21, 2020 · 3 comments
Closed

[CUDA] Compilation for NVPTX target doesn't support -fsycl-device-code-split #1560

bader opened this issue Apr 21, 2020 · 3 comments
Assignees
@AidanBeltonS
Labels
compiler Compiler related issue cuda CUDA back-end enhancement New feature or request

Comments

@bader
Copy link
Contributor

bader commented Apr 21, 2020

Only off value for -fsycl-device-code-split is compatible with -fsycl-targets= set for NVIDIA CUDA API.

Should we add support for -fsycl-device-code-split?

Revealed by KhronosGroup/SYCL-CTS#51.
@bader bader added the cuda CUDA back-end label Apr 21, 2020
@bader
Copy link
Contributor Author

bader commented Apr 21, 2020

@Naghasan, FYI.

@bader bader added the enhancement New feature or request label Jun 5, 2021
@AerialMantis AerialMantis added the compiler Compiler related issue label Aug 31, 2021
@AidanBeltonS
Copy link
Contributor

NVPTX64 support for module splitting has been added with #4107
llvm-test-suite has DeviceCodeSplit/split-per-kernel.cpp passing but failing with DeviceCodeSplit/split-per-source-main.cpp failing.

Investigation showed that failure was due to the CUDA plugin being unable to find the kernel names in the binary.
This is because the current implementation uses a regex search, which cannot create a list of names for piProgramGetInfo with module splitting per source.

Proposed fix: #4565
Add piProgramHasKernel to PI and use cuModuleGetFunction and avoid creating a list of function names owned by a program with a regex search.

A secondary pull request to llvm-test-suite will be made once #4565 passes to enable DeviceCodeSplit/split-per-kernel.cpp and DeviceCodeSplit/split-per-source-main.cpp for CUDA and HIP backends.

@AerialMantis
Copy link
Contributor

We believe this issue to be resolved by merging #4599, so we're closing the ticket.

bb-sycl pushed a commit that referenced this issue Jun 13, 2022
@jeanPerier @clementval
[flang] Avoid raising a TODO in fir.boxproc rewrite when not needed (#…
a370a4f 
…1560)

The pass was raising TODOs when a function both had a fir.boxproc<> argument
and a fir.type<> argument (even if the fir.type<> did not contain a
fir.boxproc itself).

Prevent the TODO from firing when a fir.type<> does not actually contain
a fir.boxproc. Add the location for the remaining TODO (it will be
needed when procedure pointer components are supported in lowering).

FYI, I actually tried to just implement the TODO, but I there is  a funny
issue. When creating the new fir::RecordType, since the name and context
are the same as the type being translated, fir::RecordType:get just
returns the existing type, and there is no way to change it (finalize()
does nothing since it is already finalized). So this will require to add
the ability to mutate the existing type, and I am not sure what are the
MLIR constraints here, so I escaped and left the TODO for that case.

This patch is part of the upstreaming effort from fir-dev branch.

Reviewed By: jeanPerier, PeteSteinfeld

Differential Revision: https://reviews.llvm.org/D127633

Co-authored-by: Jean Perier <jperier@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AidanBeltonS AidanBeltonS

Labels
compiler Compiler related issue cuda CUDA back-end enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bader @AerialMantis @AidanBeltonS