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

No target is specified in the test, so should OMPVV_TEST_OFFLOADING macro for offloading be removed or left alone? #740

Open
andrewkallai opened this issue Jun 12, 2023 · 2 comments
Assignees
Labels
5.1 Related to spec version 5.1 enhancement New feature or request test suggestion Recommends a new test

Comments

@andrewkallai
Copy link
Collaborator

@andrewkallai andrewkallai self-assigned this Jun 12, 2023
@andrewkallai andrewkallai added test suggestion Recommends a new test 5.1 Related to spec version 5.1 labels Jun 12, 2023
@tob2
Copy link
Collaborator

tob2 commented Jun 13, 2023

Before commit a563b9e the following message was printed on Fortran, unless that check was present:
Test ... passed on the device
which was confusing because of "device"; one workaround was to add the "OMPVV_TEST_OFFLOADING;" check, the other is the fix above.

For C/C++, the OMPVV_REPORT message does not mention "on host/device" unless OMPVV_TEST_OFFLOADING or OMPVV_TEST_AND_SET_OFFLOADING was called, avoiding the confusion.

However, I assume the main reason for the proliferation of OMPVV_TEST_OFFLOADING is that the project started by offloading testing and then old files where taken as template for new files. — Therefore, the danger is to use a non-offloading example as template for offloading ...


Missing macros — the following files do offloading but have neither of the two offloading macros

Unnecessary macros — the following files do not do offloading but have either of the two offloading macros

@spophale spophale added the enhancement New feature or request label Jun 27, 2023
@spophale
Copy link
Collaborator

Thank you @tob2! This is very useful for cleaning up. You are right, Fortran codes were using the macro differently than the C codes and once we changed what it means, there were unintended (hopefully benign) consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.1 Related to spec version 5.1 enhancement New feature or request test suggestion Recommends a new test
Projects
None yet
Development

No branches or pull requests

3 participants