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

restrict keyword used as function name #687

Closed
adegomme opened this issue Nov 13, 2024 · 3 comments · Fixed by #689
Closed

restrict keyword used as function name #687

adegomme opened this issue Nov 13, 2024 · 3 comments · Fixed by #689

Comments

@adegomme
Copy link

adegomme commented Nov 13, 2024

I was trying to compile with nvhpc compilers, and there was an error, which I think could easily be fixed.
In 5e6a135 function intersect_with was renamed to restrict. This causes issues with the nvidia compilers which don't like the "restrict" language keyword to be used for other purposes.

example of error:
"/var/tmp/ddc/include/ddc/discrete_domain.hpp", line 176: error: expected a ")"
KOKKOS_FUNCTION constexpr auto restrict(DiscreteDomain<ODDims...> const& odomain) const

is it possible to rename this function (to its previous name) ?

@tpadioleau
Copy link
Member

Hi, thank you for reporting this issue. It seems to me completely possible.

That said do we agree that nvhpc is wrong to reserve this keyword ? I mean other compilers usually reserve __restrict/__restrict__ ?

@adegomme
Copy link
Author

adegomme commented Nov 13, 2024

AFAIU, restrict keyword is in the C standard, but in C++ it's compiler-dependant, as it's not a standard feature. So indeed they should probably have avoided to block a dictionary word, and the others did that.
The fact that the compiler is unable to properly warn or give a meaningful error message is also a problem, you have to guess.

@tpadioleau
Copy link
Member

It was pointed to me that, in practice, we should also avoid to use C symbols as some C++ compilers are also able to compile C code. So I am in favor of renaming this function.

Do you need this compiler ? The CI only tests nvcc but not nvhpc. Unfortunately we do not have runtime tests but we run them manually on Nvidia clusters.

@tpadioleau tpadioleau linked a pull request Nov 27, 2024 that will close this issue
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 a pull request may close this issue.

2 participants