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

Rework BOOST_COMP_HIP and BOOST_LANG_HIP #1974

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

j-stephan
Copy link
Member

Currently BOOST_COMP_HIP can only detect whether we are compiling with clang targeting HIP. This PR adds a version detection so we can have checks like these:

#if BOOST_COMP_HIP >= BOOST_VERSION_NUMBER(5, 5, 0)

@fwyzard
Copy link
Contributor

fwyzard commented Jun 21, 2023

There is already BOOST_LANG_HIP some 20 lines above.

@j-stephan
Copy link
Member Author

From what I understand that is used to detect if we are compiling a HIP file while BOOST_COMP_HIP is used to detect whether the compiler is HIP-capable. But I'm not fully sure why we have all the different HIP checks.

@psychocoderHPC
Copy link
Member

As @fwyzard pointed out the VERSION numbers have strange behaviour, please see an older discussion #1914 (comment)

@j-stephan
Copy link
Member Author

Okay. I think a lot of (my) confusion comes from the different behaviour between CUDA and HIP:

  • BOOST_LANG_CUDA is defined when nvcc or clang encounter a *.cu file.
  • BOOST_COMP_NVCC is defined when nvcc is used as a compiler for either *.cpp or *.cu files.
  • BOOST_COMP_HIP is defined when hip-clang is used as a compiler for a *.cpp (or *.cu?) file.
  • There is no real trigger for BOOST_LANG_HIP since there are no *.hip source files. Right now this is just another (convoluted) check boiling down to the same as BOOST_COMP_HIP.
  • What @fwyzard wanted in the February PR is an HIP API version check. This is not related to compilers / languages at all.

My suggestion: Get rid of BOOST_LANG_HIP and maybe introduce something like ALPAKA_API_{CUDA; HIP} to check for the API versions of the CUDA/HIP runtimes.

@j-stephan j-stephan changed the title Improve BOOST_COMP_HIP Rework BOOST_COMP_HIP and BOOST_LANG_HIP Aug 25, 2023
Comment on lines -20 to -21
// HIP defines "abort()" as "{asm("trap;");}", which breaks some kernels
# undef abort
Copy link
Member Author

Choose a reason for hiding this comment

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

This was problematic when we targeted CUDA devices through HIP. Since we no longer do this we can remove this.

Comment on lines -23 to -26
# if defined(BOOST_LANG_CUDA) && BOOST_LANG_CUDA
# undef BOOST_LANG_CUDA
# define BOOST_LANG_CUDA BOOST_VERSION_NUMBER_NOT_AVAILABLE
# endif
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above.

include/alpaka/core/BoostPredef.hpp Outdated Show resolved Hide resolved
# if defined(__HIPCC__) // We are using a HIP-capable compiler; could be hcc, hip-clang or nvcc
# include <hip/hip_version.h>
// HIP doesn't give us a patch level for the last entry, just a gitdate
# define BOOST_COMP_HIP BOOST_VERSION_NUMBER(HIP_VERSION_MAJOR, HIP_VERSION_MINOR, 0)
Copy link
Member Author

Choose a reason for hiding this comment

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

Essentially, BOOST_COMP_HIP and BOOST_LANG_HIP do the same thing since there is no special HIP language we could test for. If we use a HIP-capable compiler we assume everything it touches is a "HIP source file".

include/alpaka/core/BoostPredef.hpp Outdated Show resolved Hide resolved
include/alpaka/core/BoostPredef.hpp Outdated Show resolved Hide resolved
@j-stephan j-stephan force-pushed the hip_predef branch 2 times, most recently from 4dcb3ee to 8971e1e Compare August 28, 2023 07:28
@j-stephan j-stephan force-pushed the hip_predef branch 2 times, most recently from c9c25e8 to d4a5964 Compare August 29, 2023 15:21
// https://github.com/ROCm-Developer-Tools/HIP/blob/master/docs/markdown/hip_porting_guide.md#compiler-defines-summary
#if !defined(BOOST_LANG_HIP)
# if defined(__HIPCC__) && (defined(__CUDACC__) || defined(__HIP__))
# include <hip/hip_runtime.h>
Copy link
Member Author

Choose a reason for hiding this comment

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

This was actually the reason for the CI failures. __forceinline__ is not a HIP keyword but defined somewhere in the header files included by hip_runtime.h.

@j-stephan
Copy link
Member Author

@psychocoderHPC Ping!

@psychocoderHPC psychocoderHPC merged commit ecf917f into alpaka-group:develop Sep 1, 2023
21 checks passed
@j-stephan j-stephan deleted the hip_predef branch September 8, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants