HIP/CUDA Gen: Use correct block size bounds in non-tensor dynamic dispatch calculations

[zatkins-dev]

Purpose:

Removes hard coded constants for block size bounds with dynamically queried values for both HIP and CUDA gen backend non-tensor kernels.

Also, fixes a bug in the field reuse code in HIP.

Closes:

LLM/GenAI Disclosure:

None.

By submitting this PR, the author certifies to its contents as described by the Developer's Certificate of Origin.
Please follow the Contributing Guidelines for all PRs.

[zatkins-dev]

See #1994 for my thoughts on how to automatically catch these failures moving forward.

[zatkins-dev]

Also, note that we have to use hipModuleOccupancyMaxPotentialBlockSize, not hipOccupancyMaxPotentialBlockSize (like the CUDA equivalent cudaOccupancyMaxPotentialBlockSize), as HIP requires the functions passed to the latter to be compiled in advance using hipcc/hipcxx.

[zatkins-dev]

Also, should we use a more sophisticated algorithm for the HIP block and grid sizes, like we do for CUDA? It seems like the limitation was in not knowing the optimal minimum grid size and block sizes, which we now have access to.

[jeremylt]

We need to do profiling here probably, because I think this has serious perf implications? @nbeams and @YohannDudouit know way better than I

[zatkins-dev]

We need to do profiling here probably, because I think this has serious perf implications?

FWIW -- The current code always falls back for Nitsche operators in Ratel, regardless of quadrature/solution order. This is a sizable perf improvement just by virtue of actually supporting gen in those cases. Not sure about general non-tensor, but I imagine this will fix some issues there too.

[jeremylt]

Good point. I think a combo of profiling and expert opinion probably is still appropriate to make sure we are appropriately slicing the available resources?

[nbeams]

My recollection: Back when we added the launch bounds to the HIP kernels, it (adding launch bounds) was recommended as best practice by our AMD liaisons for CEED, for performance. And also, an equivalent to cuOccupancyMaxPotentialBlockSize was not available for AMD GPUs until more recently (see Jed's/my comments in ROCm/hip#924).

But now that something like the CUDA routine is available, I guess the question is whether or not its (hopefully better) guess for occupancy is more important than the launch bounds or not. Of course since we have to compile the kernel first, if we either add launch bounds prematurely or go back and add launch bounds after calling the max occupancy routine, there is a chance it could change the kernel produced in a way that would alter the occupancy calculation. I really have no idea how much the launch bounds matter for recent ROCm and on the latest AMD GPUs, but it would be interesting to check.

[jeremylt]

Searching around, I think there should not be a performance impact as long as we don't have any loops governed by block size (which I think we shouldn't)

[zatkins-dev]

That's a new clang-cuda issue, I can look at it more tomorrow but truly have no idea what it means. Pretty sure it's unrelated to this PR though

[jeremylt]

I think its a clang/cuda compatibility issue. For Cuda 13.2 we really want a newer Clang than 21. I'm running Clang 23 on my current machine

[zatkins-dev]

Oh weird -- this job is supposed to be using Cuda 12.9, not 13.2

[jeremylt]

Yea, I think its something about what version of Cuda that Rust is picking up as the default. Odd that this only became an issue with the latest Rust nightly, so dunno what's up with that

[zatkins-dev]

Yea, I think its something about what version of Cuda that Rust is picking up as the default. Odd that this only became an issue with the latest Rust nightly, so dunno what's up with that

I think I have a fix for it #1993. We probably should've been specifying the CUDA dir anyway

[jeremylt]

since the rebase will reset my approval, I'll approve once this is rebased