[1/2] gdb: remove some GCC version checks

Message ID 20240221021939.5213-1-simon.marchi@efficios.com
State New
Headers
Series [1/2] gdb: remove some GCC version checks |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Simon Marchi Feb. 21, 2024, 2:19 a.m. UTC
  Since we now require C++17, and therefore gcc >= 9, it's no longer
useful to have gcc version checks for older gcc version.

Change-Id: I3a87baa03c475f2b847b422b16b69c5ff7215b54
---
 gdb/nat/x86-gcc-cpuid.h              | 17 -----------------
 gdb/unittests/enum-flags-selftests.c |  6 ------
 gdbserver/tracepoint.h               |  4 ----
 gdbsupport/common-defs.h             |  9 ---------
 4 files changed, 36 deletions(-)


base-commit: 67db6ada6370f24d344e91c2add203735292534c
  

Comments

Pedro Alves Feb. 21, 2024, 3:03 p.m. UTC | #1
On 2024-02-21 02:19, Simon Marchi wrote:

> 
> diff --git a/gdb/nat/x86-gcc-cpuid.h b/gdb/nat/x86-gcc-cpuid.h
> index dfd12587d813..b2eda44a2294 100644
> --- a/gdb/nat/x86-gcc-cpuid.h
> +++ b/gdb/nat/x86-gcc-cpuid.h
> @@ -195,7 +195,6 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
>  
>  #ifndef __x86_64__
>    /* See if we can use cpuid.  On AMD64 we always can.  */
> -#if __GNUC__ >= 3
>    __asm__ ("pushf{l|d}\n\t"
>  	   "pushf{l|d}\n\t"
>  	   "pop{l}\t%0\n\t"
> @@ -208,22 +207,6 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
>  	   "popf{l|d}\n\t"
>  	   : "=&r" (__eax), "=&r" (__ebx)
>  	   : "i" (0x00200000));
> -#else
> -/* Host GCCs older than 3.0 weren't supporting Intel asm syntax
> -   nor alternatives in i386 code.  */
> -  __asm__ ("pushfl\n\t"
> -	   "pushfl\n\t"
> -	   "popl\t%0\n\t"
> -	   "movl\t%0, %1\n\t"
> -	   "xorl\t%2, %0\n\t"
> -	   "pushl\t%0\n\t"
> -	   "popfl\n\t"
> -	   "pushfl\n\t"
> -	   "popl\t%0\n\t"
> -	   "popfl\n\t"
> -	   : "=&r" (__eax), "=&r" (__ebx)
> -	   : "i" (0x00200000));
> -#endif
>  
>    if (!((__eax ^ __ebx) & 0x00200000))
>      return 0;

It would be better IMO to avoid local changes to this file, especially if they're not
really needed:

/*
 * Helper cpuid.h file copied from gcc-6.0.0.  Code in gdb should not
                       ^^^^^^^^^^^^^^^^^^^^^
 * include this directly, but pull in x86-cpuid.h and use that func.
 */

At some point, someone may want to pull a newer version from GCC, and local changes
just make that a little more difficult.

> diff --git a/gdbserver/tracepoint.h b/gdbserver/tracepoint.h
> index 8b232324d2ec..6369e91aa276 100644
> --- a/gdbserver/tracepoint.h
> +++ b/gdbserver/tracepoint.h
> @@ -38,11 +38,7 @@ void initialize_tracepoint (void);
>  #if defined _WIN32 || defined __CYGWIN__
>  # define EXPORTED_SYMBOL __declspec (dllexport)
>  #else
> -# if __GNUC__ >= 4
>  #  define EXPORTED_SYMBOL __attribute__ ((visibility ("default")))

That "#  define" line should be "reindented".

> -# else
> -#  define EXPORTED_SYMBOL
> -# endif
>  #endif
  
Simon Marchi Feb. 21, 2024, 4:45 p.m. UTC | #2
On 2/21/24 10:03, Pedro Alves wrote:
> On 2024-02-21 02:19, Simon Marchi wrote:
> 
>>
>> diff --git a/gdb/nat/x86-gcc-cpuid.h b/gdb/nat/x86-gcc-cpuid.h
>> index dfd12587d813..b2eda44a2294 100644
>> --- a/gdb/nat/x86-gcc-cpuid.h
>> +++ b/gdb/nat/x86-gcc-cpuid.h
>> @@ -195,7 +195,6 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
>>  
>>  #ifndef __x86_64__
>>    /* See if we can use cpuid.  On AMD64 we always can.  */
>> -#if __GNUC__ >= 3
>>    __asm__ ("pushf{l|d}\n\t"
>>  	   "pushf{l|d}\n\t"
>>  	   "pop{l}\t%0\n\t"
>> @@ -208,22 +207,6 @@ __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
>>  	   "popf{l|d}\n\t"
>>  	   : "=&r" (__eax), "=&r" (__ebx)
>>  	   : "i" (0x00200000));
>> -#else
>> -/* Host GCCs older than 3.0 weren't supporting Intel asm syntax
>> -   nor alternatives in i386 code.  */
>> -  __asm__ ("pushfl\n\t"
>> -	   "pushfl\n\t"
>> -	   "popl\t%0\n\t"
>> -	   "movl\t%0, %1\n\t"
>> -	   "xorl\t%2, %0\n\t"
>> -	   "pushl\t%0\n\t"
>> -	   "popfl\n\t"
>> -	   "pushfl\n\t"
>> -	   "popl\t%0\n\t"
>> -	   "popfl\n\t"
>> -	   : "=&r" (__eax), "=&r" (__ebx)
>> -	   : "i" (0x00200000));
>> -#endif
>>  
>>    if (!((__eax ^ __ebx) & 0x00200000))
>>      return 0;
> 
> It would be better IMO to avoid local changes to this file, especially if they're not
> really needed:
> 
> /*
>  * Helper cpuid.h file copied from gcc-6.0.0.  Code in gdb should not
>                        ^^^^^^^^^^^^^^^^^^^^^
>  * include this directly, but pull in x86-cpuid.h and use that func.
>  */
> 
> At some point, someone may want to pull a newer version from GCC, and local changes
> just make that a little more difficult.

I didn't realize this was copied from gcc, despite having "gcc" in the
name.  I'll remove this bit from v2.

>> diff --git a/gdbserver/tracepoint.h b/gdbserver/tracepoint.h
>> index 8b232324d2ec..6369e91aa276 100644
>> --- a/gdbserver/tracepoint.h
>> +++ b/gdbserver/tracepoint.h
>> @@ -38,11 +38,7 @@ void initialize_tracepoint (void);
>>  #if defined _WIN32 || defined __CYGWIN__
>>  # define EXPORTED_SYMBOL __declspec (dllexport)
>>  #else
>> -# if __GNUC__ >= 4
>>  #  define EXPORTED_SYMBOL __attribute__ ((visibility ("default")))
> 
> That "#  define" line should be "reindented".

Done.

Simon
  

Patch

diff --git a/gdb/nat/x86-gcc-cpuid.h b/gdb/nat/x86-gcc-cpuid.h
index dfd12587d813..b2eda44a2294 100644
--- a/gdb/nat/x86-gcc-cpuid.h
+++ b/gdb/nat/x86-gcc-cpuid.h
@@ -195,7 +195,6 @@  __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 
 #ifndef __x86_64__
   /* See if we can use cpuid.  On AMD64 we always can.  */
-#if __GNUC__ >= 3
   __asm__ ("pushf{l|d}\n\t"
 	   "pushf{l|d}\n\t"
 	   "pop{l}\t%0\n\t"
@@ -208,22 +207,6 @@  __get_cpuid_max (unsigned int __ext, unsigned int *__sig)
 	   "popf{l|d}\n\t"
 	   : "=&r" (__eax), "=&r" (__ebx)
 	   : "i" (0x00200000));
-#else
-/* Host GCCs older than 3.0 weren't supporting Intel asm syntax
-   nor alternatives in i386 code.  */
-  __asm__ ("pushfl\n\t"
-	   "pushfl\n\t"
-	   "popl\t%0\n\t"
-	   "movl\t%0, %1\n\t"
-	   "xorl\t%2, %0\n\t"
-	   "pushl\t%0\n\t"
-	   "popfl\n\t"
-	   "pushfl\n\t"
-	   "popl\t%0\n\t"
-	   "popfl\n\t"
-	   : "=&r" (__eax), "=&r" (__ebx)
-	   : "i" (0x00200000));
-#endif
 
   if (!((__eax ^ __ebx) & 0x00200000))
     return 0;
diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c
index 853ebca37823..607b8ac66a64 100644
--- a/gdb/unittests/enum-flags-selftests.c
+++ b/gdb/unittests/enum-flags-selftests.c
@@ -253,10 +253,8 @@  CHECK_VALID (true,  EF,   true ? RE () : EF ())
 
 CHECK_VALID (true,  int,  true ? EF () : EF2 ())
 CHECK_VALID (true,  int,  true ? EF2 () : EF ())
-#if GCC_VERSION >= 5003 || defined __clang__
 CHECK_VALID (true,  int,  true ? EF () : RE2 ())
 CHECK_VALID (true,  int,  true ? RE2 () : EF ())
-#endif
 
 /* Same, but with an unsigned enum.  */
 
@@ -264,10 +262,8 @@  typedef unsigned int uns;
 
 CHECK_VALID (true,  uns,  true ? EF () : UEF ())
 CHECK_VALID (true,  uns,  true ? UEF () : EF ())
-#if GCC_VERSION >= 5003 || defined __clang__
 CHECK_VALID (true,  uns,  true ? EF () : URE ())
 CHECK_VALID (true,  uns,  true ? URE () : EF ())
-#endif
 
 /* Unfortunately this can't work due to the way C++ computes the
    return type of the ternary conditional operator.  int isn't
@@ -279,10 +275,8 @@  CHECK_VALID (true,  uns,  true ? URE () : EF ())
      error: operands to ?: have different types ‘enum_flags<RE>’ and ‘int’
    Confirmed to work with gcc 4.9, 5.3 and clang 3.7.
 */
-#if GCC_VERSION >= 4009 || defined __clang__
 CHECK_VALID (false, void, true ? EF () : 0)
 CHECK_VALID (false, void, true ? 0 : EF ())
-#endif
 
 /* Check that the ++/--/<</>>/<<=/>>= operators are deleted.  */
 
diff --git a/gdbserver/tracepoint.h b/gdbserver/tracepoint.h
index 8b232324d2ec..6369e91aa276 100644
--- a/gdbserver/tracepoint.h
+++ b/gdbserver/tracepoint.h
@@ -38,11 +38,7 @@  void initialize_tracepoint (void);
 #if defined _WIN32 || defined __CYGWIN__
 # define EXPORTED_SYMBOL __declspec (dllexport)
 #else
-# if __GNUC__ >= 4
 #  define EXPORTED_SYMBOL __attribute__ ((visibility ("default")))
-# else
-#  define EXPORTED_SYMBOL
-# endif
 #endif
 
 /* Use these to make sure the functions and variables the IPA needs to
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index 8ec559e9b680..6120719480b8 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -187,17 +187,8 @@ 
 #undef ATTRIBUTE_NONNULL
 #define ATTRIBUTE_NONNULL(m)
 
-#if GCC_VERSION >= 3004
 #define ATTRIBUTE_UNUSED_RESULT __attribute__ ((__warn_unused_result__))
-#else
-#define ATTRIBUTE_UNUSED_RESULT
-#endif
-
-#if (GCC_VERSION > 4000)
 #define ATTRIBUTE_USED __attribute__ ((__used__))
-#else
-#define ATTRIBUTE_USED
-#endif
 
 #include "libiberty.h"
 #include "pathmax.h"