Hi Andrew, all,
On 01/12/2022 13:45, Andrew Stubbs wrote:
On 01/12/2022 11:10, Paul-Antoine Arras wrote:
+      if (TARGET_FIJI)                                                         \ +    builtin_define ("__FIJI__");                                           \ +      else if (TARGET_VEGA10)                                                  \ +    builtin_define ("__VEGA10__");                                         \ +      else if (TARGET_VEGA20)                                                  \ +    builtin_define ("__VEGA20__");                                         \ +      else if (TARGET_GFX908)                                                  \ +    builtin_define ("__GFX908__");                                         \ +      else if (TARGET_GFX90a)                                                  \ +    builtin_define ("__GFX90a__");                                         \
+  } while (0)


I don't think it makes sense to say __VEGA10__ when the user asked for -march=gfx900.

This whole naming thing is a bit of a mess already, so I think we'd do better to either keep the same names throughout or match what LLVM does (since it got to these first).

Please use "__gfx900__" etc. (lower case).

[...]

P.S. If you want to split the patch into the GCN bits and the bits that depend on metadirectives then we can apply the first part to mainline right away.

I believe this patch addresses your comments regarding the GCN bits.

The new builtins are consistent with the LLVM naming convention (lower case, canonical name). For gfx803, I also kept '__fiji__' to be consistent with -march=fiji.

Is it OK for mainline?

Thanks,
--
PA
From 238e8e131741fc962fe87482d1e9a6eb1252c75c Mon Sep 17 00:00:00 2001
From: Paul-Antoine Arras <p...@codesourcery.com>
Date: Thu, 1 Dec 2022 15:09:54 +0100
Subject: [PATCH] amdgcn: Add preprocessor builtins for every processor type

Provide a specific builtin for each possible value of '-march'.

gcc/ChangeLog:

        * config/gcn/gcn-opts.h (TARGET_FIJI): -march=fiji.
        (TARGET_VEGA10): -march=gfx900.
        (TARGET_VEGA20): -march=gfx906.
        (TARGET_GFX908): -march=gfx908.
        (TARGET_GFX90a): -march=gfx90a.
        * config/gcn/gcn.h (TARGET_CPU_CPP_BUILTINS): Define a builtin that 
uniquely maps to '-march'.
---
 gcc/config/gcn/gcn-opts.h |  6 ++++++
 gcc/config/gcn/gcn.h      | 40 +++++++++++++++++++++++++--------------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git gcc/config/gcn/gcn-opts.h gcc/config/gcn/gcn-opts.h
index b62dfb45f59..b54eae79faf 100644
--- gcc/config/gcn/gcn-opts.h
+++ gcc/config/gcn/gcn-opts.h
@@ -27,6 +27,12 @@ enum processor_type
   PROCESSOR_GFX90a
 };
 
+#define TARGET_FIJI (gcn_arch == PROCESSOR_FIJI)
+#define TARGET_VEGA10 (gcn_arch == PROCESSOR_VEGA10)
+#define TARGET_VEGA20 (gcn_arch == PROCESSOR_VEGA20)
+#define TARGET_GFX908 (gcn_arch == PROCESSOR_GFX908)
+#define TARGET_GFX90a (gcn_arch == PROCESSOR_GFX90a)
+
 /* Set in gcn_option_override.  */
 extern enum gcn_isa {
   ISA_UNKNOWN,
diff --git gcc/config/gcn/gcn.h gcc/config/gcn/gcn.h
index 38f7212db59..1cc5981d904 100644
--- gcc/config/gcn/gcn.h
+++ gcc/config/gcn/gcn.h
@@ -16,20 +16,32 @@
 
 #include "config/gcn/gcn-opts.h"
 
-#define TARGET_CPU_CPP_BUILTINS()      \
-  do                                   \
-    {                                  \
-      builtin_define ("__AMDGCN__");   \
-      if (TARGET_GCN3)                 \
-       builtin_define ("__GCN3__");    \
-      else if (TARGET_GCN5)            \
-       builtin_define ("__GCN5__");    \
-      else if (TARGET_CDNA1)           \
-       builtin_define ("__CDNA1__");   \
-      else if (TARGET_CDNA2)           \
-       builtin_define ("__CDNA2__");   \
-    }                                  \
-  while(0)
+#define TARGET_CPU_CPP_BUILTINS()                                              
\
+  do                                                                           
\
+    {                                                                          
\
+      builtin_define ("__AMDGCN__");                                           
\
+      if (TARGET_GCN3)                                                         
\
+       builtin_define ("__GCN3__");                                           \
+      else if (TARGET_GCN5)                                                    
\
+       builtin_define ("__GCN5__");                                           \
+      else if (TARGET_CDNA1)                                                   
\
+       builtin_define ("__CDNA1__");                                          \
+      else if (TARGET_CDNA2)                                                   
\
+       builtin_define ("__CDNA2__");                                          \
+      if (TARGET_FIJI)                                                         
\
+       {                                                                      \
+         builtin_define ("__fiji__");                                         \
+         builtin_define ("__gfx803__");                                       \
+       }                                                                      \
+      else if (TARGET_VEGA10)                                                  
\
+       builtin_define ("__gfx900__");                                         \
+      else if (TARGET_VEGA20)                                                  
\
+       builtin_define ("__gfx906__");                                         \
+      else if (TARGET_GFX908)                                                  
\
+       builtin_define ("__gfx908__");                                         \
+      else if (TARGET_GFX90a)                                                  
\
+       builtin_define ("__gfx90a__");                                         \
+  } while (0)
 
 /* Support for a compile-time default architecture and tuning.
    The rules are:
-- 
2.31.1

Reply via email to