Re: [PATCH] nvptx: bump default to PTX 4.1

[Tom de Vries <tdevries@suse.de>]

On 1/5/22 11:33, Andrew Stubbs wrote:
> On 05/01/2022 10:24, Tom de Vries wrote:
>> On 12/21/21 12:33, Andrew Stubbs wrote:
>>> On 20/12/2021 15:58, Andrew Stubbs wrote:
>>>> In order to support the %dynamic_smem_size PTX feature is is 
>>>> necessary to bump the minimum supported PTX version from 3.1 (~2013) 
>>>> to 4.1 (~2014).
>>>
>>> Tobias has pointed out, privately, that the default version is both 
>>> documented and encoded in the -mptx option, so I need to fix that too.
>>>
>>> This patch adds -mptx=4.1, sets it as the default, and updates the 
>>> documentation accordingly.
>>>
>>> The -mptx=3.1 option is kept for backwards compatibility as an alias 
>>> for 4.1. There's no point in actually allowing 3.1 as any program 
>>> linked against libgomp will fail (and that's all offloading programs).
>>>
>>> OK for stage 1?
>>
>> Just keep -mptx=3.1 as is, and add -mptx=4.1.
>>
>> AFAIU, there's actually only one file required to have -mptx=4.1, the 
>> one using %dynamic_smem_size.  Since it's somewhat cumbersome to add 
>> flags for a single file, how about:
>> ...
>> diff --git a/libgomp/configure.tgt b/libgomp/configure.tgt
>> index d4f1e741b5a..92242697f24 100644
>> --- a/libgomp/configure.tgt
>> +++ b/libgomp/configure.tgt
>> @@ -162,6 +162,7 @@ case "${target}" in
>>
>>     nvptx*-*-*)
>>          config_path="nvptx accel"
>> +       XCFLAGS="$XCFLAGS -mptx=4.1"
>>          ;;
>>
>>     *-*-rtems*)
>> ...
>> ?
> 
> There shouldn't be any need for that as long as the default is correct.

There is no need to change the default, as long as we use this.

> In any case, I'm no expert but doesn't the deferred assembly thing mean 
> that the whole project needs to have the correct minimum setting?
> 

If your question is whether ptx modules with .version 3.1 and 4.1 can be 
linked together, then the answer seems to be yes.

> If the NVPTX maintainer prefers I can leave the 3.1 meaning actually 
> 3.1, but that will break any makefiles or build scripts that exist in 
> the wild and happen to specify 3.1 explicitly (not that I know any 
> reason why they would).

I think you're trying to support a scenario we shouldn't support, by 
making things unclear.

Thanks,
- Tom