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

Jump to bottom

Add a test (expected to fail) for calls to device function in different translation units #39

Closed
wants to merge 5 commits into from
Closed

Add a test (expected to fail) for calls to device function in different translation units #39

wants to merge 5 commits into from

Conversation

koparasy
Copy link
Collaborator

When calling a proteus annotated kernel that calls an external device function with rdc proteus fails.

AMD:

$ ./tests/gpu/device_function.HIP.rdc
ERROR @ /usr/workspace/koparasy/proteus/lib/JitEngineDeviceHIP.cpp:169 ->  HIPRTC_ERROR_LINKING
Aborted (core dumped)

CUDA:


ERROR @ /usr/workspace/koparasy/proteus/lib/JitEngineDeviceCUDA.cpp:57 ->  device kernel image is invalid
Aborted

@koparasy koparasy mentioned this pull request Nov 26, 2024
Closed
@koparasy koparasy changed the title Add a test (expected to fail) for calls external device function Add a test (expected to fail) for calls to device function in different translation units Nov 26, 2024
ggeorgakoudis
ggeorgakoudis reviewed Nov 26, 2024
View reviewed changes
tests/gpu/CMakeLists.txt
Comment on lines +76 to +83
# Code to pull only lit instructions from first source file
list(LENGTH source num_source_files)
if(${num_source_files} GREATER 1)
list(GET source 0 lit_instructions)
add_test(NAME ${exe}.${lang}.rdc COMMAND ${LIT} -vv -D EXT=${lang}.rdc ${lit_instructions})
else()
add_test(NAME ${exe}.${lang}.rdc COMMAND ${LIT} -vv -D EXT=${lang}.rdc ${source})
endif()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this? We already have a multi-file test (file1_kernel.cpp, file2_kernel.cpp) and lit picks up checks from the first file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On the rdc tests you do not have multi-file tests and are not taking into account the ARGN function parameter.

Now that both functions have an extra argument (will_fail) I bundle all sources together as a list on the second argument.

tests/gpu/CMakeLists.txt Outdated Show resolved Hide resolved
tests/gpu/main_code.cpp Outdated Show resolved Hide resolved
tests/gpu/main_code.cpp Outdated Show resolved Hide resolved
tests/gpu/device_code.cpp Outdated Show resolved Hide resolved
@koparasy
Copy link
Collaborator Author

koparasy commented Dec 3, 2024 •
edited
Loading

@ggeorgakoudis I addressed your comments and I added one more test.

The additional test generates a library from device code (when building the library the pass is turned off). The test links in with that code and executes it. This currently fails because of the original rdc issue we have. Once this branch lands I believe the test will manifest one more issue we need to fix.

@ggeorgakoudis ggeorgakoudis mentioned this pull request Dec 4, 2024
Closed
@koparasy
Copy link
Collaborator Author

koparasy commented Dec 9, 2024

@ggeorgakoudis should we close this?

@ggeorgakoudis
Copy link
Collaborator

Superseded #48

@ggeorgakoudis ggeorgakoudis deleted the tests/device-functions branch January 10, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggeorgakoudis ggeorgakoudis ggeorgakoudis left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@koparasy @ggeorgakoudis