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

Workarounds for CUDA 12.1 and 12.2 #1764

Merged
merged 7 commits into from
Aug 16, 2023
Merged

Conversation

havogt
Copy link
Contributor

@havogt havogt commented Aug 3, 2023

CUDA 12.1 and 12.2 have a problem with constexpr, e.g. in the context of CTAD, see #1766. The workaround is to do pre-C++17 make_tuple-construction or construct from a (possibly moved-from) lvalue.

CI: add GCC + CUDA 12.1/12.2 and NVHPC 23.7

@havogt
Copy link
Contributor Author

havogt commented Aug 10, 2023

launch jenkins

@havogt havogt changed the title CI: Add NVHPC 23.7 to compilation test Workarounds for CUDA 12.1 and 12.2 Aug 10, 2023
@havogt
Copy link
Contributor Author

havogt commented Aug 10, 2023

launch jenkins

@havogt
Copy link
Contributor Author

havogt commented Aug 10, 2023

launch perftests

Copy link
Contributor

@petiaccja petiaccja left a comment

Choose a reason for hiding this comment

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

Some minor cleanups, otherwise not much to do in my opinion.

One question though, why not just use gridtools::make_tuple everywhere, why a make_1_tuple?

include/gridtools/fn/column_stage.hpp Outdated Show resolved Hide resolved
include/gridtools/fn/column_stage.hpp Outdated Show resolved Hide resolved
tests/include/nvcc_workarounds.hpp Show resolved Hide resolved
tests/unit_tests/common/test_tuple.cpp Outdated Show resolved Hide resolved
@havogt
Copy link
Contributor Author

havogt commented Aug 15, 2023

Some minor cleanups, otherwise not much to do in my opinion.

One question though, why not just use gridtools::make_tuple everywhere, why a make_1_tuple?

I wanted to emphasize that this is a workaround that might be needed for users. If you see weird behavior in your code, you might compare against the example and find the problem.

@havogt
Copy link
Contributor Author

havogt commented Aug 15, 2023

launch jenkins

@havogt havogt requested a review from petiaccja August 15, 2023 07:17
tests/unit_tests/common/test_tuple.cpp Outdated Show resolved Hide resolved
@petiaccja
Copy link
Contributor

Some minor cleanups, otherwise not much to do in my opinion.
One question though, why not just use gridtools::make_tuple everywhere, why a make_1_tuple?

I wanted to emphasize that this is a workaround that might be needed for users. If you see weird behavior in your code, you might compare against the example and find the problem.

I understand, however, is this workaround ever gonna go away? (Ever = within 1-2 years.) The problem is, unless they fix their older compiler releases or we simply stop supporting those specific releases, we have to keep this workaround in the code, and then I'd rather use a proper interface that works consistently even if it's not obvious why that's used.

Co-authored-by: Péter Kardos <kardospeter1994@hotmail.com>
@havogt
Copy link
Contributor Author

havogt commented Aug 16, 2023

launch jenkins

Copy link
Contributor

@petiaccja petiaccja left a comment

Choose a reason for hiding this comment

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

As discussed, we'll keep the workaround function make_1_tuple because it's only used in the tests, so it's not visible to users. Users should be discouraged to use affected CUDA versions unless nVidia fixes the problem.

@havogt havogt merged commit 5e1011a into master Aug 16, 2023
56 checks passed
@havogt havogt deleted the test_nvhpc_237_compilation branch August 16, 2023 12:02
havogt added a commit that referenced this pull request Aug 16, 2023
CUDA 12.1 and 12.2 have a problem with constexpr, e.g. in the context of CTAD, see #1766. The workaround is to do pre-C++17 `make_tuple`-construction or construct from a (possibly moved-from) lvalue.

CI: add GCC + CUDA 12.1/12.2 and NVHPC 23.7

Co-authored-by: Péter Kardos <kardospeter1994@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants