Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3

Message ID 20200403003659.GA26283@ibm-tinman.the-meissners.org
State New
Headers show
Series
  • Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3
Related show

Commit Message

Christophe Lyon via Gcc-patches April 3, 2020, 12:36 a.m.
Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3

This patch changes the default for -mcpu=future to be -mpcrel (i.e. use
PC-relative addressing) if the ABI allows PC-relative relocations and the user
did not use either -mno-pcrel or -mno-prefixed.

I have changed the spelling of the macro to PCREL_SUPPORTED_BY_ABI (from
PCREL_SUPPORTED_BY_OS) since you pointed out it is more properly a function of
the particular ABI, rather than just an OS choice.  I have changed the various
comments to make it clearer.

I have done a bootstrap and a make check with and without the patch and there
were no regressions by adding the patch on a little endian PowerPC Linux
system.

I also tested by hand that if I use:

	-mcpu=power9
	-mcpu=future -mno-prefixed
	-mcpu=future -mno-pcrel
	-mcpu=future -mabi=elfv1	(or)
	-mcpu=future -mcmodel=large

that PC-relative addressing is not enabled by default.  Variants of this patch
have been used since December, building power8/power9 code on big endian
systems, and power8/power9/future on little endian systems.  Can I check this
into the master branch?

2020-04-02  Michael Meissner  <meissner@linux.ibm.com>

	* config/rs6000/linux64.h (PCREL_SUPPORTED_BY_ABI): Enable
	prefixed PC-relative addressing if the ABI supports it.
	* config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do not
	set OPTION_MASK_FUTURE here.
	* config/rs6000/rs6000.c (rs6000_option_override_internal): Enable
	OPTION_MASK_PREFIXED and OPTION_MASK_PREFIXED on -mcpu=future by
	default if the current ABI allows the options.


-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797

Comments

Christophe Lyon via Gcc-patches April 3, 2020, 6:32 a.m. | #1
On Thu, 2020-04-02 at 20:36 -0400, Michael Meissner via Gcc-patches
wrote:
> Enable -mpcrel on PowerPC -mcpu=future ELF v2 systems, V3

> 


Hi,

> This patch changes the default for -mcpu=future to be -mpcrel (i.e.

> use

> PC-relative addressing) if the ABI allows PC-relative relocations and

> the user

> did not use either -mno-pcrel or -mno-prefixed.

> 

> I have changed the spelling of the macro to PCREL_SUPPORTED_BY_ABI

> (from

> PCREL_SUPPORTED_BY_OS) since you pointed out it is more properly a

> function of

> the particular ABI, rather than just an OS choice.  I have changed

> the various

> comments to make it clearer.

> 

> I have done a bootstrap and a make check with and without the patch

> and there

> were no regressions by adding the patch on a little endian PowerPC

> Linux

> system.

> 

> I also tested by hand that if I use:

> 

> 	-mcpu=power9

> 	-mcpu=future -mno-prefixed

> 	-mcpu=future -mno-pcrel

> 	-mcpu=future -mabi=elfv1	(or)

> 	-mcpu=future -mcmodel=large

> 

> that PC-relative addressing is not enabled by default.  Variants of

> this patch

> have been used since December, building power8/power9 code on big

> endian

> systems, and power8/power9/future on little endian systems.  Can I

> check this

> into the master branch?

> 

> 2020-04-02  Michael Meissner  <meissner@linux.ibm.com>

> 

> 	* config/rs6000/linux64.h (PCREL_SUPPORTED_BY_ABI): Enable

> 	prefixed PC-relative addressing if the ABI supports it.


> 	* config/rs6000/rs6000-cpus.def (ISA_FUTURE_MASKS_SERVER): Do

> not

> 	set OPTION_MASK_FUTURE here.


Patch contents suggest this should be "Do not set OPTION_MASK_PREFIXED
here."

> 	* config/rs6000/rs6000.c (rs6000_option_override_internal):

> Enable

> 	OPTION_MASK_PREFIXED and OPTION_MASK_PREFIXED on -mcpu=future

> by

> 	default if the current ABI allows the options.


It's so good we enable it twice.  :-)
One of those should be s/OPTION_MASK_PREFIXED/OPTION_MASK_PCREL/


> 

> --- /tmp/IPB30g_linux64.h	2020-04-02 15:23:19.977060411 -0500

> +++ gcc/config/rs6000/linux64.h	2020-04-02 15:23:01.016474023

> -0500

> @@ -640,3 +640,11 @@ extern int dot_symbols;

>     enabling the __float128 keyword.  */

>  #undef	TARGET_FLOAT128_ENABLE_TYPE

>  #define TARGET_FLOAT128_ENABLE_TYPE 1

> +

> +/* Enable using prefixed PC-relative addressing on the 'future'

> machine if the

> +   ABI supports it.  The ELF v2 ABI only supports PC-relative

> relocations for

> +   the medium code model.  */

> +#undef  PCREL_SUPPORTED_BY_ABI


A previous comment suggested the #undef there should not be there. 
(potential for hiding or introducing a bug).


Thanks
-Will
Segher Boessenkool April 3, 2020, 4:21 p.m. | #2
Hi!

On Thu, Apr 02, 2020 at 08:36:59PM -0400, Michael Meissner wrote:
> I have changed the spelling of the macro to PCREL_SUPPORTED_BY_ABI (from

> PCREL_SUPPORTED_BY_OS) since you pointed out it is more properly a function of

> the particular ABI, rather than just an OS choice.


No, I did not, and that is a much worse name.  Please change it back.

The ABI does not determine *at all* whether PC-relative instructions can
be used.  Most ABIs do not use any PC-relative instructions themselves,
of course, but that is a very different thing.


Also please fix the problems Will pointed out (thanks!), and do post new
versions with the commit message you intend to use, please.


Segher

Patch

--- /tmp/IPB30g_linux64.h	2020-04-02 15:23:19.977060411 -0500
+++ gcc/config/rs6000/linux64.h	2020-04-02 15:23:01.016474023 -0500
@@ -640,3 +640,11 @@  extern int dot_symbols;
    enabling the __float128 keyword.  */
 #undef	TARGET_FLOAT128_ENABLE_TYPE
 #define TARGET_FLOAT128_ENABLE_TYPE 1
+
+/* Enable using prefixed PC-relative addressing on the 'future' machine if the
+   ABI supports it.  The ELF v2 ABI only supports PC-relative relocations for
+   the medium code model.  */
+#undef  PCREL_SUPPORTED_BY_ABI
+#define PCREL_SUPPORTED_BY_ABI	(TARGET_FUTURE && TARGET_PREFIXED	\
+				 && ELFv2_ABI_CHECK			\
+				 && (TARGET_CMODEL == CMODEL_MEDIUM))
--- /tmp/EHqpAk_rs6000-cpus.def	2020-04-02 15:23:19.993064282 -0500
+++ gcc/config/rs6000/rs6000-cpus.def	2020-04-02 15:23:01.016474023 -0500
@@ -75,11 +75,11 @@ 
 				 | OPTION_MASK_P8_VECTOR		\
 				 | OPTION_MASK_P9_VECTOR)
 
-/* Support for a future processor's features.  Do not enable -mpcrel until it
-   is fully functional.  */
+/* Support for a future processor's features.  We do not set the addressing
+   options OPTION_MASK_PREFIXED or OPTION_MASK_PCREL here.  Those options are
+   enabled in the function rs6000_option_override if the ABI supports them.  */
 #define ISA_FUTURE_MASKS_SERVER	(ISA_3_0_MASKS_SERVER			\
-				 | OPTION_MASK_FUTURE			\
-				 | OPTION_MASK_PREFIXED)
+				 | OPTION_MASK_FUTURE)
 
 /* Flags that need to be turned off if -mno-future.  */
 #define OTHER_FUTURE_MASKS	(OPTION_MASK_PCREL			\
--- /tmp/Zayhhc_rs6000.c	2020-04-02 15:23:20.009068153 -0500
+++ gcc/config/rs6000/rs6000.c	2020-04-02 15:23:01.020474991 -0500
@@ -4020,6 +4020,11 @@  rs6000_option_override_internal (bool gl
       rs6000_isa_flags &= ~OPTION_MASK_FLOAT128_HW;
     }
 
+  /* Enable -mprefixed by default on 64-bit 'future' systems.  */
+  if (TARGET_FUTURE && TARGET_POWERPC64
+      && (rs6000_isa_flags_explicit & OPTION_MASK_PREFIXED) == 0)
+    rs6000_isa_flags |= OPTION_MASK_PREFIXED;
+
   /* -mprefixed (and hence -mpcrel) requires -mcpu=future.  */
   if (TARGET_PREFIXED && !TARGET_FUTURE)
     {
@@ -4181,6 +4186,14 @@  rs6000_option_override_internal (bool gl
       rs6000_isa_flags &= ~OPTION_MASK_PCREL;
     }
 
+#ifdef PCREL_SUPPORTED_BY_ABI
+  /* If the ABI has support for PC-relative relocations, enable it by
+     default.  */
+  if (PCREL_SUPPORTED_BY_ABI
+      && (rs6000_isa_flags_explicit & OPTION_MASK_PCREL) == 0)
+    rs6000_isa_flags |= OPTION_MASK_PCREL;
+#endif
+
   if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
     rs6000_print_isa_options (stderr, 0, "after subtarget", rs6000_isa_flags);