CUDA Max Blocks Fix

[davideberius]

Added a fix for the case where the requested number of CUDA threads requires too many registers and results in a maximum number of CUDA blocks of 0.

Signed-off-by: David Eberius davideberius@gmail.com

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (develop@1b8c262). Click here to learn what that means.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             develop      #612   +/-   ##
===========================================
  Coverage           ?   98.663%           
===========================================
  Files              ?        65           
  Lines              ?      1272           
  Branches           ?         0           
===========================================
  Hits               ?      1255           
  Misses             ?        17           
  Partials           ?         0

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b8c262...8cf5011. Read the comment docs.

[codecov]

Codecov Report

Merging #612 into develop will decrease coverage by 0.001%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           develop      #612       +/-   ##
=============================================
- Coverage   98.664%   98.663%   -0.002%     
=============================================
  Files           65        65               
  Lines         1273      1272        -1     
=============================================
- Hits          1256      1255        -1     
  Misses          17        17

Impacted Files	Coverage Δ
include/RAJA/policy/openmp/scan.hpp	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa9b82e...82b56e2. Read the comment docs.

[rhornung67]

@ajkunen and @MrBurmark please look over this PR and weigh in. Thanks.

[ajkunen]

Looks good to me

[ajkunen]

actually @MrBurmark says that we should be using the cached occupancy calculator that we are using elsewhere, I'll let him comment on this

[rhornung67]

Yeah, that's why I wanted you and @MrBurmark to weigh in. We need to make sure we aren't adding to runtime overhead, if we can avoid it.

[davideberius]

The code path that determines the recommended threads is only executed if both num_blocks and num_threads are <= 0. The comments suggest that this means that both blocks and threads are determined at runtime. In my case, I'm hitting the logic for 'determine blocks at runtime and determine threads at compile time'.

In this case, the number of threads is not based on what can fit on the card with this kernel, so the cudaOccupancyMaxPotentialBlockSize would only be called once in the code path I am hitting.

[davideberius]

It might be worth verifying that the compile-time chosen num_threads will fit at the point that recommended_threads is assigned.

[MrBurmark]

The overhead concern is running the cudaOccupancy functions each time the kernel is hit in the user code instead of just the first time.

[MrBurmark]

actual_threads should not be changed at this point if num_threads was specified at compile-time.

[MrBurmark]

It makes sense to have CudaLaunchHelper::max_threads calculate an actual max threads in the runtime case. Perhaps it could error if the max threads from the occupancy calculator is too low in the compile-time case so we can avoid this error in the future. A test for these cases should be added as well.

[davideberius]

The problem arises when the compile time number of threads is too high, which I think should be taken into account when max_threads is calculated.

[MrBurmark]

I agree, how about you move the occupancy calculation for max_threads into the run-time case in max_threads so there is something reasonable there. In the compile-time case there isn't anything RAJA can do but check that what the user specified is possible to do and error out in a reasonable way if it isn't.

[davideberius]

The thing that confuses me about the whole compile/runtime decision is that I as the user didn't specify anything about the number of threads in my policies, so how am I always hitting the case where threads are 'determined at compile time'?

[MrBurmark]

Looks like I was forgetting how part of this all works so I'll go over the process in some detail.
The default policy is defined here.

RAJA/include/RAJA/policy/cuda/kernel/CudaKernel.hpp

Line 158 in 6b0e35d

using CudaKernel = CudaKernelFixed<1024, EnclosedStmts...>;

All cudaKernel policies end up instantiating a cuda_launch<async, num_blocks, num_threads> class to specify their num_threads and num_blocks.

RAJA/include/RAJA/policy/cuda/kernel/CudaKernel.hpp

Line 62 in 6b0e35d

struct cuda_launch {};

These "compile-time" num_threads and num_blocks are upper bounds if specified.
The lower bounds come from the policies inside the cudaKernel.
For example cudaKernel<Tile<0, tile_fixed<16>, cuda_block_x_loop, For<0, cuda_thread_x_direct, ...>>> requires at least 16 threads in the x direction and at least 1 block in the x direction. These requirements are calulated here.

RAJA/include/RAJA/policy/cuda/kernel/CudaKernel.hpp

Line 420 in 6b0e35d

LaunchDims launch_dims = executor_t::calculateDimensions(data);

[davideberius]

@MrBurmark, so the upper bound used for these kernels in the compile time case is not so much based on the kernel as it is on the definition of the policies? So, kernels with high register usage slip through the cracks.

So, my question then becomes: where should this fix be implemented? I currently have it at the spot where a failure is about to occur, however I think it would be better to have when calculating max_threads if possible.

[MrBurmark]

@davideberius Yes the upper bound in the compile time case is from the policy. Indeed the upper bound may be too high for higher register usage kernels and that was not being checked.
I agree that the upper bound should be fixed when calculating max_threads. The thing that is concerning to me is that that occupancy calculator function is maximizing overall occupancy instead of giving the max possible threads per block. Also I'm concerned that whatever is decided in max_threads could be less than min_threads.
Wherever the fix is implemented we should also be able to catch failure and print a reasonable error message.

[davideberius]

I've been looking for a CUDA function that determines the number of registers required by a kernel in order to help determine a maximum number of threads, but I haven't found one yet.

[MrBurmark]

The adjusted_threads should be corrected if less than min_threads

[MrBurmark]

Thinking about this more I have convinced myself that the real solution to this problem is to switch the default policy to use the occupancy calculator so that num_threads is unspecified. This way the initial recommended value comes from the occupancy calculator instead of changing it to use that later. When using the specified num_threads policies the kernel should either launch with that number of threads or fail to launch and we should be catching that failure and printing a better error message.

[MrBurmark]

This should use the fixed kernel launch function if num_threads is greater than zero.
internal::CudaKernelLauncherFixed<num_threads, Data, executor_t>
I broke this when I consolidated the CudaLaunchHelper implementation into a single code base.

[MrBurmark]

After talking with @ajkunen the intended behavior when num_threads is specified at compile time is for that to be a max_threads.
This should never have failed like it did because the global function should have launch_bounds applied, but that is not being used because I broke that part when I worked on this last.
I'll fix that.