[gomp4] adjust num_gangs and add a diagnostic for unsupported num_workers

On 02/15/2017 01:29 PM, Thomas Schwinge wrote:
> On Mon, 13 Feb 2017 08:58:39 -0800, Cesar Philippidis <cesar@codesourcery.com> wrote:

>> @@ -952,25 +958,30 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, void **devaddrs,
>>        CUdevice dev = nvptx_thread()->ptx_dev->dev;
>>        /* 32 is the default for known hardware.  */
>>        int gang = 0, worker = 32, vector = 32;
>> -	  CUdevice_attribute cu_tpb, cu_ws, cu_mpc, cu_tpm;
>> +      CUdevice_attribute cu_tpb, cu_ws, cu_mpc, cu_tpm, cu_rf, cu_sm;
>>  
>>        cu_tpb = CU_DEVICE_ATTRIBUTE_MAX_THREADS_PER_BLOCK;
>>        cu_ws = CU_DEVICE_ATTRIBUTE_WARP_SIZE;
>>        cu_mpc = CU_DEVICE_ATTRIBUTE_MULTIPROCESSOR_COUNT;
>>        cu_tpm  = CU_DEVICE_ATTRIBUTE_MAX_THREADS_PER_MULTIPROCESSOR;
>> +      cu_rf  = CU_DEVICE_ATTRIBUTE_MAX_REGISTERS_PER_MULTIPROCESSOR;
>> +      cu_sm  = CU_DEVICE_ATTRIBUTE_MAX_SHARED_MEMORY_PER_MULTIPROCESSOR;
>>  
>>        if (cuDeviceGetAttribute (&block_size, cu_tpb, dev) == CUDA_SUCCESS
>>  	  && cuDeviceGetAttribute (&warp_size, cu_ws, dev) == CUDA_SUCCESS
>>  	  && cuDeviceGetAttribute (&dev_size, cu_mpc, dev) == CUDA_SUCCESS
>> -	      && cuDeviceGetAttribute (&cpu_size, cu_tpm, dev)  == CUDA_SUCCESS)
>> +	  && cuDeviceGetAttribute (&cpu_size, cu_tpm, dev) == CUDA_SUCCESS
>> +	  && cuDeviceGetAttribute (&rf_size, cu_rf, dev)  == CUDA_SUCCESS
>> +	  && cuDeviceGetAttribute (&sm_size, cu_sm, dev)  == CUDA_SUCCESS)
> 
> Trying to compile this on CUDA 5.5/331.113, I run into:
> 
>     [...]/source-gcc/libgomp/plugin/plugin-nvptx.c: In function 'nvptx_exec':
>     [...]/source-gcc/libgomp/plugin/plugin-nvptx.c:970:16: error: 'CU_DEVICE_ATTRIBUTE_MAX_REGISTERS_PER_MULTIPROCESSOR' undeclared (first use in this function)
>            cu_rf  = CU_DEVICE_ATTRIBUTE_MAX_REGISTERS_PER_MULTIPROCESSOR;
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     [...]/source-gcc/libgomp/plugin/plugin-nvptx.c:970:16: note: each undeclared identifier is reported only once for each function it appears in
>     [...]/source-gcc/libgomp/plugin/plugin-nvptx.c:971:16: error: 'CU_DEVICE_ATTRIBUTE_MAX_SHARED_MEMORY_PER_MULTIPROCESSOR' undeclared (first use in this function)
>            cu_sm  = CU_DEVICE_ATTRIBUTE_MAX_SHARED_MEMORY_PER_MULTIPROCESSOR;
>                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> For reference, please see the code handling
> CU_DEVICE_ATTRIBUTE_MAX_REGISTERS_PER_MULTIPROCESSOR in the trunk version
> of the nvptx_open_device function.

ACK. While this change is fairly innocuous, it might be too invasive for
GCC 7.1. Maybe we can backport it to 7.1?

> And then, I don't specifically have a problem with discontinuing CUDA 5.5
> support, and require 6.5, for example, but that should be a conscious
> decision.

We should probably ditch CUDA 5.5. In fact, according to trunk's cuda.h,
it requires version 8.0.

Alex, are you using CUDA 5.5 in your environment?

>> @@ -980,8 +991,6 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, void **devaddrs,
>>  	 matches the hardware configuration.  Logical gangs are
>>  	 scheduled onto physical hardware.  To maximize usage, we
>>  	 should guess a large number.  */
>> -	  if (default_dims[GOMP_DIM_GANG] < 1)
>> -	    default_dims[GOMP_DIM_GANG] = gang ? gang : 1024;
> 
> That's "bad", because a non-zero "default_dims[GOMP_DIM_GANG]" (also
> known as "default_dims[0]") is used to decide whether to enter this whole
> code block, and with that assignment removed, every call of the
> nvptx_exec function will now re-do all this GOMP_OPENACC_DIM parsing,
> cuDeviceGetAttribute calls, computations, and so on.  (See "GOMP_DEBUG=1"
> output.)

Good point. I made neutral values (e.g. '-' arguments as negative one).

> I think this whole code block should be moved into the nvptx_open_device
> function, to have it executed once when the device is opened -- after
> all, all these are per-device attributes.  (So, it's actually
> conceptually incorrect to have this done only once in the nvptx_exec
> function, given that this data then is used in the same process by/for
> potentially different hardware devices.)

Yeah, that's a better place. All of those hardware attributes are now
stored in ptx_device.

> And, one could argue that the GOMP_OPENACC_DIM parsing conceptually
> belongs into generic libgomp code, instead of the nvptx plugin.  (But
> that aspect can be cleaned up later: currently, the nvptx plugin is the
> only one supporting/using it.)
> 
>>        /* The worker size must not exceed the hardware.  */
>>        if (default_dims[GOMP_DIM_WORKER] < 1
>>  	  || (default_dims[GOMP_DIM_WORKER] > worker && gang))
>> @@ -998,9 +1007,56 @@ nvptx_exec (void (*fn), size_t mapnum, void **hostaddrs, void **devaddrs,
>>      }
>>    pthread_mutex_unlock (&ptx_dev_lock);
>>  
>> +  int reg_used = -1;  /* Dummy value.  */
>> +  cuFuncGetAttribute (&reg_used, CU_FUNC_ATTRIBUTE_NUM_REGS, function);
> 
> Why need to assign this "dummy value"?

Yeah, it's unnecessary.

> Now, per my understanding, this "function attribute" must be constant for
> all calls of that function.  So, shouldn't this be queried once, after
> "linking" the code (link_ptx function)?  And indeed, that's what the
> trunk version of the nvptx plugin's GOMP_OFFLOAD_load_image function is
> doing.

Yeah, I did that. The "num_regs" attribute is now stored in
targ_fn_descriptor.

>> +
>> +      if (dims[GOMP_DIM_WORKER] > threads_per_block)
>> +	GOMP_PLUGIN_fatal ("The Nvidia accelerator has insufficient resources "
>> +			   "to launch '%s'; recompile the program with "
>> +			   "'num_workers = %d' on that offloaded region or "
>> +			   "'-fopenacc-dim=-:%d'.\n",
>> +			   targ_fn->launch->fn, threads_per_block,
>> +			   threads_per_block);
>>      }
> 
> ACK -- until we come up with a better solution.

Keep in mind that setting num_workers statically, i.e. as a constant
value, really helps improve performance. When I allowed num_workers to
remain a non-constant variable, I observed a 2.5x slowdown in
cloverleaf. That's an isolated case (which dependent on a specific data
set), but it's still something to consider.

I applied this patch to gomp-4_0-branch.

Do I need to lock access to nvptx_thread? If so, I can go back and fix
it later.

Cesar

[gomp4] adjust num_gangs and add a diagnostic for unsupported num_workers

Commit Message

Comments

Patch