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?

[zatkins-dev]

We could also leave the cap on 512 by doing this:

Suggested change

	CeedInt elems_per_block = CeedIntMin(cuda_data->device_prop.maxThreadsDim[2], CeedIntMax(max_threads_per_block / data->thread_1d, 1));
	CeedInt elems_per_block = CeedIntMin(cuda_data->device_prop.maxThreadsDim[2], CeedIntMax(CeedIntMin(max_threads_per_block, 512) / data->thread_1d, 1));

That would ensure the previous behavior for the standard non-tensor elements while preventing the elems_per_block from being too high for the weird cases.

[jeremylt]

That's probably a good safeguard here

[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.