amdgcn: Change -m(no-)xnack to -mxnack=(on,off,any)

Message ID afcb40d0-8ad7-e311-1981-344d855231f1@codesourcery.com
State Committed
Headers
Series amdgcn: Change -m(no-)xnack to -mxnack=(on,off,any) |

Commit Message

Tobias Burnus May 26, 2023, 2:58 p.m. UTC
  (Update the syntax of the amdgcn commandline option in anticipation of later patches;
while -m(no-)xnack is in mainline since r12-2396-gaad32a00b7d2b6 (for PR100208),
-mxsnack (contrary to -msram-ecc) is currently mostly a stub for later patches
and is documented as such in invoke.texi. Thus, this change should have no (or
only a minimal) affect on users.)


GCC currently supports for GCN -mxnack / -mno-xnack arguments, matching
+xnack and -xnack when passed to the LLVM linker. However, since V4 the latter
supports three states, besides on/off there is now also unspecified. That matches
the semantic of sram(-)ecc, which GCC already implements as 'on'/'off' and 'any'.

Cf. https://llvm.org/docs/AMDGPUUsage.html#target-features
and https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/AMD-GCN-Options.html


The attached patch uses the sram-ecc flag syntax now also for xnack.
Note that currently only 'no' is supported which is ensured via a 'sorry'.
Hence, the default is 'no'. I assume we want to change the default once
XNACK is working - therefore, the documentation does only states the current
default as a comment.

The changes were picked from the patch "amdgcn: Support XNACK mode" at
- https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597991.html
- OG12 0229066ecb24421d48e3e0d56f31c30cc1affdab
- OG13 cbc3dd01de8788587a2b641efcb838058303b5ab
but only includes all changes related to the commandline option changes,
excluding the other changes like those to isns.

It additionally updates invoke.texi (using the wording from -msram-ecc).
(I actually encountered this issue because of the non-updated manual.)

Tested with full bootstrap, regtesting running, but not expecting surprised.
OK for mainline?

Tobias

PS: For FIJI, "" is passed – that's ensured by NO_XNACK in the ASM_SPEC
and the 'switch' later in output_file_start (unchanged), otherwise 'xnack-'
is used (via the default in gcn.opt for the compiler and via XNACKOPT for
the command line.)
-----------------
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
  

Comments

Andrew Stubbs May 26, 2023, 3:05 p.m. UTC | #1
OK.

Andrew
On 26/05/2023 15:58, Tobias Burnus wrote:
> (Update the syntax of the amdgcn commandline option in anticipation of 
> later patches;
> while -m(no-)xnack is in mainline since r12-2396-gaad32a00b7d2b6 (for 
> PR100208),
> -mxsnack (contrary to -msram-ecc) is currently mostly a stub for later 
> patches
> and is documented as such in invoke.texi. Thus, this change should have 
> no (or
> only a minimal) affect on users.)
> 
> 
> GCC currently supports for GCN -mxnack / -mno-xnack arguments, matching
> +xnack and -xnack when passed to the LLVM linker. However, since V4 the 
> latter
> supports three states, besides on/off there is now also unspecified. 
> That matches
> the semantic of sram(-)ecc, which GCC already implements as 'on'/'off' 
> and 'any'.
> 
> Cf. 
> https://llvm.org/docs/AMDGPUUsage.html#target-features>> and 
> https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/AMD-GCN-Options.html>> 
> 
> The attached patch uses the sram-ecc flag syntax now also for xnack.
> Note that currently only 'no' is supported which is ensured via a 'sorry'.
> Hence, the default is 'no'. I assume we want to change the default once
> XNACK is working - therefore, the documentation does only states the 
> current
> default as a comment.
> 
> The changes were picked from the patch "amdgcn: Support XNACK mode" at
> - 
> https://gcc.gnu.org/pipermail/gcc-patches/2022-July/597991.html>> - OG12 0229066ecb24421d48e3e0d56f31c30cc1affdab
> - OG13 cbc3dd01de8788587a2b641efcb838058303b5ab
> but only includes all changes related to the commandline option changes,
> excluding the other changes like those to isns.
> 
> It additionally updates invoke.texi (using the wording from -msram-ecc).
> (I actually encountered this issue because of the non-updated manual.)
> 
> Tested with full bootstrap, regtesting running, but not expecting 
> surprised.
> OK for mainline?
> 
> Tobias
> 
> PS: For FIJI, "" is passed – that's ensured by NO_XNACK in the ASM_SPEC
> and the 'switch' later in output_file_start (unchanged), otherwise 'xnack-'
> is used (via the default in gcn.opt for the compiler and via XNACKOPT for
> the command line.)
> -----------------
> 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
  

Patch

amdgcn: Change -m(no-)xnack to -mxnack=(on,off,any)

Since object code target ID V4, xnack has the values unspecified, '+' and '-',
which with this commit is represented in GCC as 'any', 'on', and 'off',
following the precidence for 'sram(-)ecc' and -msram-ecc=.

The current default was 'no' and is now 'off'; however, once XNACK is
implemented, the default should be probably 'any'.

This commit updates the commandline options to permit the new tristate and
updates the documentation. As the feature itself is currently not really
supported in GCC, the change should not affect real-world users.

The XNACK feature allows memory load instructions to restart safely following
a page-miss interrupt.  This is useful for shared-memory devices, like APUs,
and to implement OpenMP Unified Shared Memory.

2023-05-26  Andrew Stubbs  <ams@codesourcery.com>
	    Tobias Burnus  <tobias@codesourcery.com>

	* config/gcn/gcn-hsa.h (XNACKOPT): New macro.
	(ASM_SPEC): Use XNACKOPT.
	* config/gcn/gcn-opts.h (enum sram_ecc_type): Rename to ...
	(enum hsaco_attr_type): ... this, and generalize the names.
	(TARGET_XNACK): New macro.
	* config/gcn/gcn.cc (gcn_option_override): Update to sorry for all
	but -mxnack=off.
	(output_file_start): Update xnack handling.
	(gcn_hsa_declare_function_name): Use TARGET_XNACK.
	* config/gcn/gcn.opt (-mxnack): Add the "on/off/any" syntax.
	(sram_ecc_type): Rename to ...
	(hsaco_attr_type: ... this.)
	* config/gcn/mkoffload.c (SET_XNACK_ANY): New macro.
	(TEST_XNACK): Delete.
	(TEST_XNACK_ANY): New macro.
	(TEST_XNACK_ON): New macro.
       	(main): Support the new -mxnack=on/off/any syntax.
	* doc/invoke.texi (-mxnack): Update for new syntax.

 gcc/config/gcn/gcn-hsa.h    |  6 +++++-
 gcc/config/gcn/gcn-opts.h   | 10 ++++++----
 gcc/config/gcn/gcn.cc       | 19 +++++++++++--------
 gcc/config/gcn/gcn.opt      | 20 ++++++++++----------
 gcc/config/gcn/mkoffload.cc | 21 +++++++++++++++------
 gcc/doc/invoke.texi         | 14 ++++++++------
 6 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/gcc/config/gcn/gcn-hsa.h b/gcc/config/gcn/gcn-hsa.h
index 51b0509a8a6..0b5610bbcbe 100644
--- a/gcc/config/gcn/gcn-hsa.h
+++ b/gcc/config/gcn/gcn-hsa.h
@@ -82,11 +82,15 @@  extern unsigned int gcn_local_sym_hash (const char *name);
    configuration.  The name of the attribute also changed.  */
 #define SRAMOPT "msram-ecc=on:-mattr=+sramecc;msram-ecc=off:-mattr=-sramecc"
 
+/* Replace once XNACK is supported:
+   #define XNACKOPT "mxnack=on:-mattr=+xnack;mxnack=off:-mattr=-xnack"  */
+#define XNACKOPT "!mnack=*:-mattr=-xnack;mnack=*:-mattr=-xnack"
+
 /* Use LLVM assembler and linker options.  */
 #define ASM_SPEC  "-triple=amdgcn--amdhsa "  \
 		  "%:last_arg(%{march=*:-mcpu=%*}) " \
 		  "%{!march=*|march=fiji:--amdhsa-code-object-version=3} " \
-		  "%{" NO_XNACK "mxnack:-mattr=+xnack;:-mattr=-xnack} " \
+		  "%{" NO_XNACK XNACKOPT "}" \
 		  "%{" NO_SRAM_ECC SRAMOPT "} " \
 		  "-filetype=obj"
 #define LINK_SPEC "--pie --export-dynamic"
diff --git a/gcc/config/gcn/gcn-opts.h b/gcc/config/gcn/gcn-opts.h
index cd1b2ecca96..f780a7c17fe 100644
--- a/gcc/config/gcn/gcn-opts.h
+++ b/gcc/config/gcn/gcn-opts.h
@@ -54,11 +54,13 @@  extern enum gcn_isa {
 #define TARGET_M0_LDS_LIMIT (TARGET_GCN3)
 #define TARGET_PACKED_WORK_ITEMS (TARGET_CDNA2_PLUS)
 
-enum sram_ecc_type
+#define TARGET_XNACK (flag_xnack != HSACO_ATTR_OFF)
+
+enum hsaco_attr_type
 {
-  SRAM_ECC_OFF,
-  SRAM_ECC_ON,
-  SRAM_ECC_ANY
+  HSACO_ATTR_OFF,
+  HSACO_ATTR_ON,
+  HSACO_ATTR_ANY
 };
 
 #endif
diff --git a/gcc/config/gcn/gcn.cc b/gcc/config/gcn/gcn.cc
index 007730da2f9..efb7211d54e 100644
--- a/gcc/config/gcn/gcn.cc
+++ b/gcc/config/gcn/gcn.cc
@@ -157,8 +157,10 @@  gcn_option_override (void)
 	acc_lds_size = 32768;
     }
 
-  /* The xnack option is a placeholder, for now.  */
-  if (flag_xnack)
+  /* The xnack option is a placeholder, for now.  Before removing, update
+     gcn-hsa.h's XNACKOPT, gcn.opt's mxnack= default init+descr, and
+     invoke.texi's default description.  */
+  if (flag_xnack != HSACO_ATTR_OFF)
     sorry ("XNACK support");
 }
 
@@ -6162,11 +6164,12 @@  static void
 output_file_start (void)
 {
   /* In HSACOv4 no attribute setting means the binary supports "any" hardware
-     configuration.  In GCC binaries, this is true for SRAM ECC, but not
-     XNACK.  */
-  const char *xnack = (flag_xnack ? ":xnack+" : ":xnack-");
-  const char *sram_ecc = (flag_sram_ecc == SRAM_ECC_ON ? ":sramecc+"
-			  : flag_sram_ecc == SRAM_ECC_OFF ? ":sramecc-"
+     configuration.  */
+  const char *xnack = (flag_xnack == HSACO_ATTR_ON ? ":xnack+"
+		       : flag_xnack == HSACO_ATTR_OFF ? ":xnack-"
+		       : "");
+  const char *sram_ecc = (flag_sram_ecc == HSACO_ATTR_ON ? ":sramecc+"
+			  : flag_sram_ecc == HSACO_ATTR_OFF ? ":sramecc-"
 			  : "");
 
   const char *cpu;
@@ -6210,7 +6213,7 @@  void
 gcn_hsa_declare_function_name (FILE *file, const char *name, tree)
 {
   int sgpr, vgpr;
-  bool xnack_enabled = false;
+  bool xnack_enabled = TARGET_XNACK;
 
   fputs ("\n\n", file);
 
diff --git a/gcc/config/gcn/gcn.opt b/gcc/config/gcn/gcn.opt
index c5c32bdc833..36c2b535284 100644
--- a/gcc/config/gcn/gcn.opt
+++ b/gcc/config/gcn/gcn.opt
@@ -81,23 +81,23 @@  Wopenacc-dims
 Target Var(warn_openacc_dims) Warning
 Warn about invalid OpenACC dimensions.
 
-mxnack
-Target Var(flag_xnack) Init(0)
-Compile for devices requiring XNACK enabled. Default off.
-
 Enum
-Name(sram_ecc_type) Type(enum sram_ecc_type)
-SRAM-ECC modes:
+Name(hsaco_attr_type) Type(enum hsaco_attr_type)
+SRAM-ECC and XNACK modes:
 
 EnumValue
-Enum(sram_ecc_type) String(off) Value(SRAM_ECC_OFF)
+Enum(hsaco_attr_type) String(off) Value(HSACO_ATTR_OFF)
 
 EnumValue
-Enum(sram_ecc_type) String(on) Value(SRAM_ECC_ON)
+Enum(hsaco_attr_type) String(on) Value(HSACO_ATTR_ON)
 
 EnumValue
-Enum(sram_ecc_type) String(any) Value(SRAM_ECC_ANY)
+Enum(hsaco_attr_type) String(any) Value(HSACO_ATTR_ANY)
+
+mxnack=
+Target RejectNegative Joined ToLower Enum(hsaco_attr_type) Var(flag_xnack) Init(HSACO_ATTR_OFF)
+Compile for devices requiring XNACK enabled. Default \"off\".
 
 msram-ecc=
-Target RejectNegative Joined ToLower Enum(sram_ecc_type) Var(flag_sram_ecc) Init(SRAM_ECC_ANY)
+Target RejectNegative Joined ToLower Enum(hsaco_attr_type) Var(flag_sram_ecc) Init(HSACO_ATTR_ANY)
 Compile for devices with the SRAM ECC feature enabled, or not. Default \"any\".
diff --git a/gcc/config/gcn/mkoffload.cc b/gcc/config/gcn/mkoffload.cc
index 61bc9273077..988c12318fd 100644
--- a/gcc/config/gcn/mkoffload.cc
+++ b/gcc/config/gcn/mkoffload.cc
@@ -72,10 +72,16 @@ 
 
 #define SET_XNACK_ON(VAR) VAR = ((VAR & ~EF_AMDGPU_FEATURE_XNACK_V4) \
 				 | EF_AMDGPU_FEATURE_XNACK_ON_V4)
+#define SET_XNACK_ANY(VAR) VAR = ((VAR & ~EF_AMDGPU_FEATURE_XNACK_V4) \
+				  | EF_AMDGPU_FEATURE_XNACK_ANY_V4)
 #define SET_XNACK_OFF(VAR) VAR = ((VAR & ~EF_AMDGPU_FEATURE_XNACK_V4) \
 				  | EF_AMDGPU_FEATURE_XNACK_OFF_V4)
-#define TEST_XNACK(VAR) ((VAR & EF_AMDGPU_FEATURE_XNACK_V4) \
-			 == EF_AMDGPU_FEATURE_XNACK_ON_V4)
+#define TEST_XNACK_ANY(VAR) ((VAR & EF_AMDGPU_FEATURE_XNACK_V4) \
+			     == EF_AMDGPU_FEATURE_XNACK_ANY_V4)
+#define TEST_XNACK_ON(VAR) ((VAR & EF_AMDGPU_FEATURE_XNACK_V4) \
+			    == EF_AMDGPU_FEATURE_XNACK_ON_V4)
+#define TEST_XNACK_OFF(VAR) ((VAR & EF_AMDGPU_FEATURE_XNACK_V4) \
+			     == EF_AMDGPU_FEATURE_XNACK_OFF_V4)
 
 #define SET_SRAM_ECC_ON(VAR) VAR = ((VAR & ~EF_AMDGPU_FEATURE_SRAMECC_V4) \
 				    | EF_AMDGPU_FEATURE_SRAMECC_ON_V4)
@@ -907,9 +913,11 @@  main (int argc, char **argv)
 	fPIC = true;
       else if (strcmp (argv[i], "-fpic") == 0)
 	fpic = true;
-      else if (strcmp (argv[i], "-mxnack") == 0)
+      else if (strcmp (argv[i], "-mxnack=on") == 0)
 	SET_XNACK_ON (elf_flags);
-      else if (strcmp (argv[i], "-mno-xnack") == 0)
+      else if (strcmp (argv[i], "-mxnack=any") == 0)
+	SET_XNACK_ANY (elf_flags);
+      else if (strcmp (argv[i], "-mxnack=off") == 0)
 	SET_XNACK_OFF (elf_flags);
       else if (strcmp (argv[i], "-msram-ecc=on") == 0)
 	SET_SRAM_ECC_ON (elf_flags);
@@ -1073,8 +1081,9 @@  main (int argc, char **argv)
       obstack_ptr_grow (&ld_argv_obstack, gcn_s2_name);
       obstack_ptr_grow (&ld_argv_obstack, "-lgomp");
       obstack_ptr_grow (&ld_argv_obstack,
-			(TEST_XNACK (elf_flags)
-			 ? "-mxnack" : "-mno-xnack"));
+			(TEST_XNACK_ON (elf_flags) ? "-mxnack=on"
+			 : TEST_XNACK_ANY (elf_flags) ? "-mxnack=any"
+			 : "-mxnack=off"));
       obstack_ptr_grow (&ld_argv_obstack,
 			(TEST_SRAM_ECC_ON (elf_flags) ? "-msram-ecc=on"
 			 : TEST_SRAM_ECC_ANY (elf_flags) ? "-msram-ecc=any"
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ee78591c73e..898a88ce33e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -20822,12 +20822,14 @@  run-time performance.  The default is 32KB when using OpenACC or OpenMP, and
 1MB otherwise.
 
 @opindex mxnack
-@item -mxnack
-Compile binaries suitable for devices with the XNACK feature enabled.  Some
-devices always require XNACK and some allow the user to configure XNACK.  The
-compiled code must match the device mode.  The default is @samp{-mno-xnack}.
-At present this option is a placeholder for support that is not yet
-implemented.
+@item -mxnack=on
+@itemx -mxnack=off
+@itemx -mxnack=any
+Compile binaries suitable for devices with the XNACK feature enabled, disabled,
+or either mode.  Some devices always require XNACK and some allow the user to
+configure XNACK.  The compiled code must match the device mode.
+@c The default is @samp{-mxnack=any}.
+At present this option is a placeholder for support that is not yet implemented.
 
 @end table