HIP Library, main branch (2025.11.13.) #1193
Conversation
krasznaa
force-pushed
the
HIPBackend-main-20251113
branch
from
08d35fa to
dcedeab
Compare
|
Hi Attila, Great to see this. |
device/hip/CMakeLists.txt
Outdated
| PUBLIC traccc::core vecmem::core | ||
| PRIVATE HIP::hiprt traccc::device_common ) | ||
|
|
||
| # # Ensure relaxed constexpr is enabled for device compilation (nvcc/hipcc) |
There was a problem hiding this comment.
Did you decide against this?
There was a problem hiding this comment.
That code was left in by mistake in that place. It was Copilot doing its own stuff there.
I rather put this sort of a thing into cmake/traccc-compiler-options-hip.cmake. We do need to make this setting, but in a more complicated way than how Copilot tried it. (As these flags are only meaningful when hipcc uses nvcc in the background. Not when it's building for an AMD backend.)
I'll do some cleanup on such leftovers today. As long as you're happy with the direction with all of this, I'll push forward.
It's mostly what I'm doing to rocThrust in this PR that you should have a look at. As that's a rather fragile part, which may affect Alpaka as well. (Not in the unit tests, but I didn't run anything more thorough.)
|
We need to have a serious conversation about the number of backends that we have now. If this PR goes in, we have four different methods of targetting AMD GPUs (HIP, Kokkos, Alpaka, SYCL), or which three (HIP, Kokkos, Alpaka) deliver exactly the same code to the ROCm compiler and can therefore, almost by definition, not lead to any meaningful performance benefits. As with all the backends for traccc, this puts the maintenance burden on the people actually developing the algorithms. We've gone from having to duplicate all our changes twice to three times to four times and now to five times. It's not a sustainable development model. |
|
Alpaka and hip tests work fine for me, without getting into bigger issues. |
krasznaa
force-pushed
the
HIPBackend-main-20251113
branch
from
dcedeab to
9abb85d
Compare
Added mostly infrastructure code to it at first, along with a clusterization algorithm. All absolutely based on the CUDA code, with very minimal changes.
krasznaa
force-pushed
the
HIPBackend-main-20251113
branch
from
9abb85d to
e410533
Compare
|
3 participants
This all started out with me trying to see if CoPilot would be able to do all of this automatically / by itself. Unfortunately that all worked very poorly, so I rather did the whole migration by hand in the end. 😦
But still, I wanted to start making a
traccc::hiplibrary finally.To start things off, I converted
traccc::cuda::clusterization_algorithminto atraccc::hip::clusterization_algorithm. Code-wise it was pretty much a copy-paste, with acuda => hipreplacement applied. The only tricky part was to convince the build system to behave more or less reliably.Which for me specifically it by now does. But I'll need some additional eyes as well. @StewMH, please have a look. Specifically at what ( did to the setup of rocThrust. As it was giving me a world of hurt. 😦
For now I didn't do anything specific to the CI setup. But (for me a bit) surprisingly
TRACCC_BUILD_HIPis already exercised by the"ALPAKA HIP SYCL"build. 😕 @StewMH, should we take things a bit more apart? The Alpaka build should really only test builds through Alpaka. I'd rather test the build of "native HIP code" in a separate CI job. Or rather, ideally we'd add 2 new CI jobs. One building the HIP code with its AMD backend, and another one using the NVIDIA backend. As one can very easily break either one of these without breaking the other. 😦