[PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tom de Vries]

Hi,

Update nvptx documentation:
- Use meaningful terms: "PTX ISA target architecture" and "PTX ISA version".
- Remove invalid claim that "ISA strings must be lower-case".
- Add missing sm_xx entries.
- Fix default ISA.
- Add march, copying misa doc.
- Declare misa an march alias.
- Add march-map.
- Fix "for given the specified" typo.

Any comments?

Thanks,
- Tom

[nvptx, doc] Update misa and mptx, add march and march-map

gcc/ChangeLog:

2022-03-29  Tom de Vries  <tdevries@suse.de>

	* doc/invoke.texi (misa, mptx): Update.
	(march, march-map): Add.

---
 gcc/doc/invoke.texi | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 554e04ecbf3a..eb2fe959e600 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -27540,18 +27540,31 @@ These options are defined for Nvidia PTX:
 Ignored, but preserved for backward compatibility.  Only 64-bit ABI is
 supported.
 
-@item -misa=@var{ISA-string}
+@item -march=@var{architecture-string}
 @opindex march
-Generate code for given the specified PTX ISA (e.g.@: @samp{sm_35}).  ISA
-strings must be lower-case.  Valid ISA strings include @samp{sm_30} and
-@samp{sm_35}.  The default ISA is sm_35.
+Generate code for the specified PTX ISA target architecture
+(e.g.@: @samp{sm_35}).  Valid architecture strings are @samp{sm_30},
+@samp{sm_35}, @samp{sm_53} @samp{sm_70}, @samp{sm_75} and
+@samp{sm_80}.  The default target architecture is sm_30.
+
+@item -misa=@var{architecture-string}
+@opindex misa
+Alias of @option{-march=}.
+
+@item -march-map=@var{architecture-string}
+@opindex march
+Select the closest available @option{-march=} value that is not more
+capable.  For instance, for @option{-march-map=sm_50} select
+@option{-march=sm_35}, and for @option{-march-map=sm_53} select
+@option{-march=sm_53}.
 
 @item -mptx=@var{version-string}
 @opindex mptx
-Generate code for given the specified PTX version (e.g.@: @samp{7.0}).
+Generate code for the specified PTX ISA version (e.g.@: @samp{7.0}).
 Valid version strings include @samp{3.1}, @samp{6.0}, @samp{6.3}, and
-@samp{7.0}.  The default PTX version is 6.0, unless a higher minimal
-version is required for specified PTX ISA via option @option{-misa=}.
+@samp{7.0}.  The default PTX version is 6.0, unless a higher version
+is required for specified PTX ISA target architecture via option
+@option{-march=}.
 
 @item -mmainkernel
 @opindex mmainkernel

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tobias Burnus]

Hi Tom,

On 29.03.22 15:39, Tom de Vries wrote:
> Any comments?
> +(e.g.@: @samp{sm_35}).  Valid architecture strings are @samp{sm_30},
> +@samp{sm_35}, @samp{sm_53} @samp{sm_70}, @samp{sm_75} and
> +@samp{sm_80}.  The default target architecture is sm_30.

Missing comma (",") between sm_53 and sm_70.

I want to note that the default is now back at sm_30;
for GCC 11 it was changed to sm_35, cf. https://gcc.gnu.org/gcc-11/changes.html
(We also need to update the wwwdocs release notes before the release, but it
can also be done after branching).

> +@item -march-map=@var{architecture-string}
> +@opindex march
> +Select the closest available @option{-march=} value that is not more
> +capable.  For instance, for @option{-march-map=sm_50} select
> +@option{-march=sm_35}, and for @option{-march-map=sm_53} select
> +@option{-march=sm_53}.

(Somehow, I am not completely happy with the wording, but, admittedly, I
don't have a better suggestion.)

Tobias


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tobias Burnus]

On 29.03.22 16:28, Tobias Burnus wrote:

> On 29.03.22 15:39, Tom de Vries wrote:
>> Any comments?

I think it would be useful to have additionally some wording for the
(new in GCC 12/new since today) macros, i.e. something like:

--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -27546,6 +27546,10 @@
  strings must be lower-case.  Valid ISA strings include @samp{sm_30} and
  @samp{sm_35}.  The default ISA is sm_35.

+This option causes the preprocessor macro @code{__PTX_SM__} to be defined
+to the architecture number multiplied by ten; for instance, for
+@samp{sm_35}, it has the value @samp{350}.
+
  @item -mptx=@var{version-string}
  @opindex mptx
  Generate code for given the specified PTX version (e.g.@: @samp{7.0}).
@@ -27553,6 +27557,10 @@
  @samp{7.0}.  The default PTX version is 6.0, unless a higher minimal
  version is required for specified PTX ISA via option @option{-misa=}.

+This option causes the preprocessor macros @code{__PTX_ISA_VERSION_MAJOR__}
+and @code{__PTX_ISA_VERSION_MINOR__} to be defined; for instance,
+for @samp{3.1} the macros have the values @samp{3} and @samp{1}, respectively.
+
  @item -mmainkernel
  @opindex mmainkernel
  Link in code for a __main kernel.  This is for stand-alone instead of

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]

On 3/29/22 16:47, Tobias Burnus wrote:
> On 29.03.22 16:28, Tobias Burnus wrote:
> 
>> On 29.03.22 15:39, Tom de Vries wrote:
>>> Any comments?
> 
> I think it would be useful to have additionally some wording for the
> (new in GCC 12/new since today) macros,

Agreed.

> i.e. something like:
> 
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -27546,6 +27546,10 @@
>   strings must be lower-case.  Valid ISA strings include @samp{sm_30} and
>   @samp{sm_35}.  The default ISA is sm_35.
> 
> +This option causes the preprocessor macro @code{__PTX_SM__} to be defined
> +to the architecture number multiplied by ten; for instance, for
> +@samp{sm_35}, it has the value @samp{350}.
> +

The macro is defined also if the option is not specified, so I think 
this formulation is not 100% clear in that aspect.  I've reformulated to 
fix that.

Also, I took out the detail of how the value is determined, since we're 
just following __CUDA_ARCH__ rather than defining our own policy.

>   @item -mptx=@var{version-string}
>   @opindex mptx
>   Generate code for given the specified PTX version (e.g.@: @samp{7.0}).
> @@ -27553,6 +27557,10 @@
>   @samp{7.0}.  The default PTX version is 6.0, unless a higher minimal
>   version is required for specified PTX ISA via option @option{-misa=}.
> 
> +This option causes the preprocessor macros 
> @code{__PTX_ISA_VERSION_MAJOR__}
> +and @code{__PTX_ISA_VERSION_MINOR__} to be defined; for instance,
> +for @samp{3.1} the macros have the values @samp{3} and @samp{1}, 
> respectively.
> +

Reformulated this as well.

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0001-nvptx-doc-Document-predefined-macros-at-march-and-mptx.patch --]
[-- Type: text/x-patch, Size: 1585 bytes --]

[nvptx, doc] Document predefined macros at march and mptx

Document predefined macros:
- __PTX_SM__ ,
- __PTX_ISA_VERSION_MAJOR__ and
- __PTX_ISA_VERSION_MINOR__ .

gcc/ChangeLog:

2022-03-29  Tom de Vries  <tdevries@suse.de>

	* doc/invoke.texi (march): Document __PTX_SM__.
	 (mptx): Document __PTX_ISA_VERSION_MAJOR__ and
	 __PTX_ISA_VERSION_MINOR__.

Co-Authored-By: Tobias Burnus <tobias@codesourcery.com>

---
 gcc/doc/invoke.texi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 43b75132c91b..09715a510b4d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -27547,6 +27547,10 @@ Generate code for the specified PTX ISA target architecture
 @samp{sm_35}, @samp{sm_53}, @samp{sm_70}, @samp{sm_75} and
 @samp{sm_80}.  The default target architecture is sm_30.
 
+This option sets the value of the preprocessor macro
+@code{__PTX_SM__}; for instance, for @samp{sm_35}, it has the value
+@samp{350}.
+
 @item -misa=@var{architecture-string}
 @opindex misa
 Alias of @option{-march=}.
@@ -27566,6 +27570,11 @@ Valid version strings include @samp{3.1}, @samp{6.0}, @samp{6.3}, and
 version is required for specified PTX ISA target architecture via
 option @option{-march=}.
 
+This option sets the values of the preprocessor macros
+@code{__PTX_ISA_VERSION_MAJOR__} and @code{__PTX_ISA_VERSION_MINOR__};
+for instance, for @samp{3.1} the macros have the values @samp{3} and
+@samp{1}, respectively.
+
 @item -mmainkernel
 @opindex mmainkernel
 Link in code for a __main kernel.  This is for stand-alone instead of

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tobias Burnus]

On 30.03.22 10:03, Tom de Vries wrote:

> On 3/29/22 16:47, Tobias Burnus wrote:
>> I think it would be useful to have additionally some wording for the
>> (new in GCC 12/new since today) macros,
[...]
> The macro is defined also if the option is not specified, so I think
> this formulation is not 100% clear in that aspect.  I've reformulated
> to fix that.
Fine. (It was a copy, paste + modify from elsewhere.)

> Also, I took out the detail of how the value is determined, since
> we're just following __CUDA_ARCH__ rather than defining our own policy.

OK. While I am not sure that it is obvious, also the example makes clear
what value to expect. Combining the two, I concur that the details
aren't required.

> Any comments?

LGTM.

Tobias


PS: Regarding the sm_30 -> sm_35 change (before in this email thread).
That was not meant to be in the the .texi file, but just as item to
remember when updating the wwwdocs / gcc-12/changes.html document.

It was/is also not completely clear to me whether there is still this
CUDA 11.x issue of not supporting sm_30 (only sm_35 and higher) or not.
I assume it still exists but is mitigated at
compiler-usage/libgomp-runtime-usage time as PTX ISA now defaults to 6.0
such that CUDA – but shouldn't it still see sm_30 instead of sm_35 in
this case?

If so, I think it will still show up when using either explicitly PTX
ISA 3.1 or when building GCC itself and all of the following holds:
nvptx-tools is installed, CUDA (in a too new version) is installed
(ptxas in $PATH) , and the the pending pull request nvptx-tools has not
been applied that ignores the non-explicit '--verify' when .target sm_xx
or PTX ISA .version is not supported by ptxas.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 3396 bytes --]

On 3/30/22 11:02, Tobias Burnus wrote:
> On 30.03.22 10:03, Tom de Vries wrote:
> 
>> On 3/29/22 16:47, Tobias Burnus wrote:
>>> I think it would be useful to have additionally some wording for the
>>> (new in GCC 12/new since today) macros,
> [...]
>> The macro is defined also if the option is not specified, so I think
>> this formulation is not 100% clear in that aspect.  I've reformulated
>> to fix that.
> Fine. (It was a copy, paste + modify from elsewhere.)
> 
>> Also, I took out the detail of how the value is determined, since
>> we're just following __CUDA_ARCH__ rather than defining our own policy.
> 
> OK. While I am not sure that it is obvious, also the example makes clear
> what value to expect. Combining the two, I concur that the details
> aren't required.
> 
>> Any comments?
> 
> LGTM.
> 
> Tobias
> 
> 
> PS: Regarding the sm_30 -> sm_35 change (before in this email thread).
> That was not meant to be in the the .texi file, but just as item to
> remember when updating the wwwdocs / gcc-12/changes.html document.
> 

I see, I misunderstood then.  FWIW, it's already added to the version in 
my sandbox.

> It was/is also not completely clear to me whether there is still this
> CUDA 11.x issue of not supporting sm_30 (only sm_35 and higher) or not.

Thanks for reminding me of this issue.

> I assume it still exists but is mitigated at
> compiler-usage/libgomp-runtime-usage time as PTX ISA now defaults to 6.0
> such that CUDA – but shouldn't it still see sm_30 instead of sm_35 in
> this case?
> 
> If so, I think it will still show up when using either explicitly PTX
> ISA 3.1 or when building GCC itself and all of the following holds:
> nvptx-tools is installed, CUDA (in a too new version) is installed
> (ptxas in $PATH) , and the the pending pull request nvptx-tools has not
> been applied that ignores the non-explicit '--verify' when .target sm_xx
> or PTX ISA .version is not supported by ptxas.


I don't think the 6.0 default has any influence (and I'll be using 
-mptx=3.1 below to make sure we run into the worst-case behaviour).

Anyway, in absence of an nvptx-tools fix I committed a work-around in 
the compiler:
...
#define ASM_SPEC "%{misa=*:-m %*; :-m sm_35}%{misa=sm_30:--no-verify}"
...

Note that this was before reverting back the default to sm_30, and I 
probably forgot to update this spot when changing the default.

So now, there are effectively two workarounds in place.

This (implicitly using sm_30) passes:
...
$ ( PATH=$PATH:~/cuda/11.6.0/bin; ./gcc.sh ~/hello.c -c -save-temps 
-Wa,--verify -mptx=3.1 )
...
because as we can see with -v, sm_35 is used to verify:
...
  ./build-gcc/gcc/as -m sm_35 --verify -o hello.o hello.s
...

This (explicitly using sm_30) passes:
...
$ ( PATH=$PATH:~/cuda/11.6.0/bin; ./gcc.sh ~/hello.c -c -save-temps 
-march=sm_30 -mptx=3.1 )
...
because as we can see with -v, the --no-verify workaround is triggered:
...
  ./build-gcc/gcc/as -m sm_30 --no-verify -o hello.o hello.s
...

But that one stops working once we use an explicit -Wa,--verify:
...
$ ( PATH=$PATH:~/cuda/11.6.0/bin; ./gcc.sh ~/hello.c -c -save-temps 
-Wa,--verify -march=sm_30 -mptx=3.1  )
ptxas fatal   : Value 'sm_30' is not defined for option 'gpu-name'
nvptx-as: ptxas returned 255 exit status
...

So, it seems using sm_35 to verify sm_30 is the most robust workaround.

I'm currently testing attached patch.

Thanks,
- Tom

[-- Attachment #2: 0001-nvptx-Fix-ASM_SPEC-workaround-for-sm_30.patch --]
[-- Type: text/x-patch, Size: 1865 bytes --]

[nvptx] Fix ASM_SPEC workaround for sm_30

Newer versions of CUDA no longer support sm_30, and nvptx-tools as
currently doesn't handle that gracefully when verifying
( https://github.com/MentorEmbedded/nvptx-tools/issues/30 ).

There's a --no-verify work-around in place in ASM_SPEC, but that one doesn't
work when using -Wa,--verify on the command line.

Use a more robust workaround: verify using sm_35 when misa=sm_30 is specified
(either implicitly or explicitly).

Tested on nvptx.

gcc/ChangeLog:

2022-03-30  Tom de Vries  <tdevries@suse.de>

	* config/nvptx/nvptx.h (ASM_SPEC): Use "-m sm_35" for -misa=sm_30.

---
 gcc/config/nvptx/nvptx.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/gcc/config/nvptx/nvptx.h b/gcc/config/nvptx/nvptx.h
index 75ac7a666b13..3b06f33032fd 100644
--- a/gcc/config/nvptx/nvptx.h
+++ b/gcc/config/nvptx/nvptx.h
@@ -29,10 +29,24 @@
 
 #define STARTFILE_SPEC "%{mmainkernel:crt0.o}"
 
-/* Default needs to be in sync with default for misa in nvptx.opt.
-   We add a default here to work around a hard-coded sm_30 default in
-   nvptx-as.  */
-#define ASM_SPEC "%{misa=*:-m %*; :-m sm_35}%{misa=sm_30:--no-verify}"
+/* Newer versions of CUDA no longer support sm_30, and nvptx-tools as
+   currently doesn't handle that gracefully when verifying
+   ( https://github.com/MentorEmbedded/nvptx-tools/issues/30 ).  Work around
+   this by verifying with sm_35 when having misa=sm_30 (either implicitly
+   or explicitly).  */
+#define ASM_SPEC				\
+  "%{"						\
+  /* Explict misa=sm_30.  */			\
+  "misa=sm_30:-m sm_35"				\
+  /* Separator.	 */				\
+  "; "						\
+  /* Catch-all.	 */				\
+  "misa=*:-m %*"				\
+  /* Separator.	 */				\
+  "; "						\
+  /* Implicit misa=sm_30.  */			\
+  ":-m sm_35"					\
+  "}"
 
 #define TARGET_CPU_CPP_BUILTINS() nvptx_cpu_cpp_builtins ()
 

Re: [PATCH][nvptx, doc] Update misa and mptx, add march and march-map

[Tom de Vries]

On 3/29/22 16:28, Tobias Burnus wrote:
> Hi Tom,
> 
> On 29.03.22 15:39, Tom de Vries wrote:
>> Any comments?
>> +(e.g.@: @samp{sm_35}).  Valid architecture strings are @samp{sm_30},
>> +@samp{sm_35}, @samp{sm_53} @samp{sm_70}, @samp{sm_75} and
>> +@samp{sm_80}.  The default target architecture is sm_30.
> 
> Missing comma (",") between sm_53 and sm_70.
> 

Ack, fixed.

> I want to note that the default is now back at sm_30;
> for GCC 11 it was changed to sm_35, cf. 
> https://gcc.gnu.org/gcc-11/changes.html

I think changes are better described in release notes.

> (We also need to update the wwwdocs release notes before the release, 
> but it
> can also be done after branching).
> 

Right, I'll follow up on your proposal from beginning of this month.

>> +@item -march-map=@var{architecture-string}
>> +@opindex march
>> +Select the closest available @option{-march=} value that is not more
>> +capable.  For instance, for @option{-march-map=sm_50} select
>> +@option{-march=sm_35}, and for @option{-march-map=sm_53} select
>> +@option{-march=sm_53}.
> 
> (Somehow, I am not completely happy with the wording, but, admittedly, I
> don't have a better suggestion.)

I feel the same, so committed as is.

Thanks,
- Tom