amdgcn: Add preprocessor builtins for every processor type

Message ID b3b7e809-537d-c2f0-c02d-b1050967edbd@codesourcery.com
State Committed
Headers
Series amdgcn: Add preprocessor builtins for every processor type |

Commit Message

Paul-Antoine Arras Dec. 1, 2022, 2:35 p.m. UTC
  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,
  

Comments

Andrew Stubbs Dec. 1, 2022, 2:42 p.m. UTC | #1
On 01/12/2022 14:35, Paul-Antoine Arras wrote:
> 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?

You need to wrap the long line in the changelog (I'm not sure it'll even 
let you push it like that), but otherwise it looks fine.

OK.

Andrew
  

Patch

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: