diff mbox series

[committed,nvptx] Choose -mptx default based on -misa

Message ID 20220208125751.GA7486@delia.home
State New
Headers show
Series [committed,nvptx] Choose -mptx default based on -misa | expand

Commit Message

Tom de Vries Feb. 8, 2022, 12:57 p.m. UTC
Hi,

While testing with driver version 390.147 I ran into the problem that it
doesn't support ptx isa version 6.3 (the new default), only 6.1.

Furthermore, using the -mptx option is a bit user-unfriendly.

Say we want to compile for sm_80.  We can use -misa=sm_80 to specify that, but
then run into errors because the default ptx version is 6.3, which doesn't
support sm_80 yet.

Address both these issues by:
- picking a default -mptx based on the active -misa, and
- ensuring that the default -mptx is at least 6.0 (instead
  of 6.3).

Also add an error in case of incompatible options like
"-misa=sm_80 -mptx=6.3":
...
cc1: error: PTX version (-mptx) needs to be at least 7.0 to support \
  selected -misa (sm_80)
...

Tested on x86_64-linux with nvptx accelerator.

Committed to trunk.

Thanks,
- Tom

[nvptx] Choose -mptx default based on -misa

gcc/ChangeLog:

2022-02-08  Tom de Vries  <tdevries@suse.de>

	PR target/104283
	* config/nvptx/nvptx-opts.h (enum ptx_version): Add PTX_VERSION_3_0
	and PTX_VERSION_4_2.
	* config/nvptx/nvptx.cc (first_ptx_version_supporting_sm)
	(default_ptx_version_option, ptx_version_to_string)
	(sm_version_to_string, handle_ptx_version_option): New function.
	(nvptx_option_override): Call handle_ptx_version_option.
	(nvptx_file_start): Use ptx_version_to_string and sm_version_to_string.
	* config/nvptx/nvptx.md (define_insn "nvptx_shuffle<mode>")
	(define_insn "nvptx_vote_ballot"): Use TARGET_PTX_6_0.
	* config/nvptx/nvptx.opt (mptx): Remove 'Init'.

---
 gcc/config/nvptx/nvptx-opts.h |   2 +
 gcc/config/nvptx/nvptx.cc     | 133 +++++++++++++++++++++++++++++++++++++-----
 gcc/config/nvptx/nvptx.md     |   4 +-
 gcc/config/nvptx/nvptx.opt    |   2 +-
 4 files changed, 122 insertions(+), 19 deletions(-)

Comments

Tom de Vries Feb. 8, 2022, 1:18 p.m. UTC | #1
On 2/8/22 13:57, Tom de Vries via Gcc-patches wrote:
> +static const char *
> +sm_version_to_string (enum ptx_isa sm)
> +{
> +  switch (sm)
> +    {
> +    case PTX_ISA_SM30:
> +      return "30";
> +    case PTX_ISA_SM35:
> +      return "35";
> +    case PTX_ISA_SM53:
> +      return "53";
> +    case PTX_ISA_SM70:
> +      return "70";

This broke the nvptx build, PTX_ISA_SM70 is not defined yet, that was 
done in another patch, that I didn't commit yet.

I'm doing a rebuild with a patch to fix this, and will commit afterwards.

Sorry for the disturbance.

Thanks,
- Tom
Tobias Burnus Feb. 8, 2022, 1:24 p.m. UTC | #2
Hi Tom,

if I understand the patch correctly, -misa=sm_53 -mptx=3.1 will ...

On 08.02.22 13:57, Tom de Vries via Gcc-patches wrote:
> Furthermore, using the -mptx option is a bit user-unfriendly.
>
> Say we want to compile for sm_80.  We can use -misa=sm_80 to specify that, but
> then run into errors because the default ptx version is 6.3, which doesn't
> support sm_80 yet.
> ...
> Also add an error in case of incompatible options like
> "-misa=sm_80 -mptx=6.3":
> ...
> cc1: error: PTX version (-mptx) needs to be at least 7.0 to support \
>    selected -misa (sm_80)

... output that -mptx needs to be at least 4.1.

I think that's okay but -mptx only supports the values 3.1, 6.3, and 7.0.

As the default is ptx = 6.3, it only occurs when both are specified in
the way shown above. Thus, we can live with that. (Misleading message
for odd corner case, only. In particular, I am not sure we really want
to add another PTX version...)

(Which points to the question of supporting multilib configurations – as
discussed before:
https://gcc.gnu.org/pipermail/gcc/2022-February/238224.html )

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
Tom de Vries Feb. 8, 2022, 1:56 p.m. UTC | #3
On 2/8/22 14:24, Tobias Burnus wrote:
> Hi Tom,
> 
> if I understand the patch correctly, -misa=sm_53 -mptx=3.1 will ...
> 
> On 08.02.22 13:57, Tom de Vries via Gcc-patches wrote:
>> Furthermore, using the -mptx option is a bit user-unfriendly.
>>
>> Say we want to compile for sm_80.  We can use -misa=sm_80 to specify 
>> that, but
>> then run into errors because the default ptx version is 6.3, which 
>> doesn't
>> support sm_80 yet.
>> ...
>> Also add an error in case of incompatible options like
>> "-misa=sm_80 -mptx=6.3":
>> ...
>> cc1: error: PTX version (-mptx) needs to be at least 7.0 to support \
>>    selected -misa (sm_80)
> 
> ... output that -mptx needs to be at least 4.1.

Um, you mean 4.2 I suppose:
...
$ ./gcc.sh ~/hello.c -misa=sm_53 -mptx=3.1
cc1: error: PTX version (-mptx) needs to be at least 4.2 to support 
selected -misa (sm_53)
...
?

> I think that's okay but -mptx only supports the values 3.1, 6.3, and 7.0.
> 

I know.  I'm sort of hoping that the new default setting will make using 
-mptx unnecessary.

> As the default is ptx = 6.3,

Well, that's no longer the case, that's one of the things that this 
patch changes.  It's by default the maximum of:
- 6.0, and
- the minimal ptx isa required by the sm version

> it only occurs when both are specified in
> the way shown above. Thus, we can live with that. (Misleading message
> for odd corner case, only. In particular, I am not sure we really want
> to add another PTX version...)
> 

Agreed, it's misleading, but I'm hoping people will just specify the sm 
version.

Thanks,
- Tom
diff mbox series

Patch

diff --git a/gcc/config/nvptx/nvptx-opts.h b/gcc/config/nvptx/nvptx-opts.h
index c754a5193ce..cc488b23720 100644
--- a/gcc/config/nvptx/nvptx-opts.h
+++ b/gcc/config/nvptx/nvptx-opts.h
@@ -31,7 +31,9 @@  enum ptx_isa
 
 enum ptx_version
 {
+  PTX_VERSION_3_0,
   PTX_VERSION_3_1,
+  PTX_VERSION_4_2,
   PTX_VERSION_6_0,
   PTX_VERSION_6_3,
   PTX_VERSION_7_0
diff --git a/gcc/config/nvptx/nvptx.cc b/gcc/config/nvptx/nvptx.cc
index 006fac8c839..1b0227a2c31 100644
--- a/gcc/config/nvptx/nvptx.cc
+++ b/gcc/config/nvptx/nvptx.cc
@@ -205,6 +205,109 @@  diagnose_openacc_conflict (bool optval, const char *optname)
     error ("option %s is not supported together with %<-fopenacc%>", optname);
 }
 
+static enum ptx_version
+first_ptx_version_supporting_sm (enum ptx_isa sm)
+{
+  switch (sm)
+    {
+    case PTX_ISA_SM30:
+      return PTX_VERSION_3_0;
+    case PTX_ISA_SM35:
+      return PTX_VERSION_3_1;
+    case PTX_ISA_SM53:
+      return PTX_VERSION_4_2;
+    case PTX_ISA_SM75:
+      return PTX_VERSION_6_3;
+    case PTX_ISA_SM80:
+      return PTX_VERSION_7_0;
+    default:
+      gcc_unreachable ();
+    }
+}
+
+static enum ptx_version
+default_ptx_version_option (void)
+{
+  enum ptx_version first
+    = first_ptx_version_supporting_sm ((enum ptx_isa) ptx_isa_option);
+
+  /* Pick a version that supports the sm.  */
+  enum ptx_version res = first;
+
+  /* Pick at least 3.1.  This has been the smallest version historically.  */
+  res = MAX (res, PTX_VERSION_3_1);
+
+  /* Pick at least 6.0, to enable using bar.warp.sync to have a way to force
+     warp convergence.  */
+  res = MAX (res, PTX_VERSION_6_0);
+
+  /* Verify that we pick a version that supports the sm.  */
+  gcc_assert (first <= res);
+  return res;
+}
+
+static const char *
+ptx_version_to_string (enum ptx_version v)
+{
+  switch (v)
+    {
+    case PTX_VERSION_3_0:
+      return "3.0";
+    case PTX_VERSION_3_1:
+      return "3.1";
+    case PTX_VERSION_4_2:
+      return "4.2";
+    case PTX_VERSION_6_0:
+      return "6.0";
+    case PTX_VERSION_6_3:
+      return "6.3";
+    case PTX_VERSION_7_0:
+      return "7.0";
+    default:
+      gcc_unreachable ();
+    }
+}
+
+static const char *
+sm_version_to_string (enum ptx_isa sm)
+{
+  switch (sm)
+    {
+    case PTX_ISA_SM30:
+      return "30";
+    case PTX_ISA_SM35:
+      return "35";
+    case PTX_ISA_SM53:
+      return "53";
+    case PTX_ISA_SM70:
+      return "70";
+    case PTX_ISA_SM75:
+      return "75";
+    case PTX_ISA_SM80:
+      return "80";
+    default:
+      gcc_unreachable ();
+    }
+}
+
+static void
+handle_ptx_version_option (void)
+{
+  if (!OPTION_SET_P (ptx_version_option))
+    {
+      ptx_version_option = default_ptx_version_option ();
+      return;
+    }
+
+  enum ptx_version first
+    = first_ptx_version_supporting_sm ((enum ptx_isa) ptx_isa_option);
+
+  if (ptx_version_option < first)
+    error ("PTX version (-mptx) needs to be at least %s to support selected"
+	   " -misa (sm_%s)", ptx_version_to_string (first),
+	   sm_version_to_string ((enum ptx_isa)ptx_isa_option));
+}
+
 /* Implement TARGET_OPTION_OVERRIDE.  */
 
 static void
@@ -212,6 +315,8 @@  nvptx_option_override (void)
 {
   init_machine_status = nvptx_init_machine_status;
 
+  handle_ptx_version_option ();
+
   /* Set toplevel_reorder, unless explicitly disabled.  We need
      reordering so that we emit necessary assembler decls of
      undeclared variables. */
@@ -5430,23 +5535,19 @@  static void
 nvptx_file_start (void)
 {
   fputs ("// BEGIN PREAMBLE\n", asm_out_file);
-  if (TARGET_PTX_7_0)
-    fputs ("\t.version\t7.0\n", asm_out_file);
-  else if (TARGET_PTX_6_3)
-    fputs ("\t.version\t6.3\n", asm_out_file);
-  else
-    fputs ("\t.version\t3.1\n", asm_out_file);
-  if (TARGET_SM80)
-    fputs ("\t.target\tsm_80\n", asm_out_file);
-  else if (TARGET_SM75)
-    fputs ("\t.target\tsm_75\n", asm_out_file);
-  else if (TARGET_SM53)
-    fputs ("\t.target\tsm_53\n", asm_out_file);
-  else if (TARGET_SM35)
-    fputs ("\t.target\tsm_35\n", asm_out_file);
-  else
-    fputs ("\t.target\tsm_30\n", asm_out_file);
+
+  fputs ("\t.version\t", asm_out_file);
+  fputs (ptx_version_to_string ((enum ptx_version)ptx_version_option),
+	 asm_out_file);
+  fputs ("\n", asm_out_file);
+
+  fputs ("\t.target\tsm_", asm_out_file);
+  fputs (sm_version_to_string ((enum ptx_isa)ptx_isa_option),
+	 asm_out_file);
+  fputs ("\n", asm_out_file);
+
   fprintf (asm_out_file, "\t.address_size %d\n", GET_MODE_BITSIZE (Pmode));
+
   fputs ("// END PREAMBLE\n", asm_out_file);
 }
 
diff --git a/gcc/config/nvptx/nvptx.md b/gcc/config/nvptx/nvptx.md
index d64dbfd8b33..7463603a0b0 100644
--- a/gcc/config/nvptx/nvptx.md
+++ b/gcc/config/nvptx/nvptx.md
@@ -1603,7 +1603,7 @@  (define_insn "nvptx_shuffle<mode>"
 		  UNSPEC_SHUFFLE))]
   ""
   {
-    if (TARGET_PTX_6_3)
+    if (TARGET_PTX_6_0)
       return "%.\\tshfl.sync%S3.b32\\t%0, %1, %2, 31, 0xffffffff;";
     else
       return "%.\\tshfl%S3.b32\\t%0, %1, %2, 31;";
@@ -1615,7 +1615,7 @@  (define_insn "nvptx_vote_ballot"
 		   UNSPEC_VOTE_BALLOT))]
   ""
   {
-    if (TARGET_PTX_6_3)
+    if (TARGET_PTX_6_0)
       return "%.\\tvote.sync.ballot.b32\\t%0, %1, 0xffffffff;";
     else
       return "%.\\tvote.ballot.b32\\t%0, %1;";
diff --git a/gcc/config/nvptx/nvptx.opt b/gcc/config/nvptx/nvptx.opt
index 6e12b1f7296..e3f65b2d0b1 100644
--- a/gcc/config/nvptx/nvptx.opt
+++ b/gcc/config/nvptx/nvptx.opt
@@ -89,5 +89,5 @@  EnumValue
 Enum(ptx_version) String(7.0) Value(PTX_VERSION_7_0)
 
 mptx=
-Target RejectNegative ToLower Joined Enum(ptx_version) Var(ptx_version_option) Init(PTX_VERSION_6_3)
+Target RejectNegative ToLower Joined Enum(ptx_version) Var(ptx_version_option)
 Specify the version of the ptx version to use.