Don't clobber CMAKE_HIP_FLAGS

[StewMH]

Needed this to get hipcc to use a different gcc toolchain, i.e.

cmake ../vecmem -DCMAKE_HIP_FLAGS="--gcc-toolchain=/cvmfs/sft.cern.ch/lcg/releases/gcc/11.3.0-ad0f5/x86_64-centos8"

instead of the system compiler (gcc8).

[krasznaa]

Hi Stewart,

The HIP compilation is probably not quite correctly set up at the moment, that I can easily believe. But at the same time, you should never set CMAKE_<LANG>_FLAGS variables from the command line like that. Even for something like CMAKE_CXX_FLAGS this is a very bad idea.

The behaviour with CMAKE_CXX_FLAGS in such a case is that whatever you specified, is the value of CMAKE_CXX_FLAGS. And any optimization, debugging, etc. flags that CMake would normally add, are removed/overwritten. (Specifying -DCMAKE_CXX_FLAGS is a good way of producing un-optimised binaries unknowingly.)

Long story short, you should use the $HIPFLAGS environment variable for specifying additional flags for your build. Just like how one is meant to use $CXXFLAGS, $CUDAFLAGS and even $SYCLFLAGS. (The latter definitely works with this project. But I don't remember testing $HIPFLAGS with this CMake code...)

Cheers, Attila

[krasznaa]

Hmm... I made some tests just now, but it seems that $HIPFLAGS was never made to work properly with our code. 😦

However... instead of putting a whole lot of effort into upgrading this project's HIP CMake code, it might finally be time to move to using CMake's own HIP code. 🤔 Since as I found just now, with CMake 3.28 they finally taught that code also how to build NVIDIA binaries.

https://cmake.org/cmake/help/latest/variable/CMAKE_HIP_PLATFORM.html

Which was the last feature that I was missing from that code. (Even if I would also prefer to have a HIP_PLATFORM target property that could choose the HIP platform target-by-target. As I noted here. But fine, we never had that in our code either.)

So we should find some good way in our CMake code to either use the imperfect code from this repository for CMake <3.28, or CMake's own code in >=3.28. 🤔

Would you be willing to give that a try Stewart?

[krasznaa]

I was too optimistic. 😦 CMake's HIP build for an NVIDIA backend still doesn't work. 😦

https://gitlab.kitware.com/cmake/cmake/-/issues/25541

So I'll be making some improvements to our own HIP language code after all... 🤔

[StewMH]

Thanks, let me check your other PR.