RFA: Fix PR 68028: LTO error when compiling PowerPC binaries with single precision floating point

Message ID 877ernpjl9.fsf@redhat.com
State New
Headers show
Series
  • RFA: Fix PR 68028: LTO error when compiling PowerPC binaries with single precision floating point
Related show

Commit Message

Nick Clifton Feb. 8, 2018, 3:49 p.m.
Hi Segher,

  OK, here is an official submission of my patch to fix PR 68028.

  I should note that Richard Guenther feels that there is a better way
  to solve the problem - by only initializing the values once - but I
  still like my solution, so I am offering it here.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2018-02-08  Nick Clifton  <nickc@redhat.com>

	* config/rs6000/rs6000.c (rs6000_option_override_internal):
        In LTO mode prefer function attributes over command line -mcpu
        setting.

Comments

Segher Boessenkool Feb. 8, 2018, 5:01 p.m. | #1
Hi Nick,

On Thu, Feb 08, 2018 at 03:49:38PM +0000, Nick Clifton wrote:
>   I should note that Richard Guenther feels that there is a better way
>   to solve the problem - by only initializing the values once - but I
>   still like my solution, so I am offering it here.

I thought you were going to do a patch like the following, to make the
e500 cores less special (they are not):


From 44c3b661ef75e57415b10ca2cec5ac6427bbed8f Mon Sep 17 00:00:00 2001
Message-Id: <44c3b661ef75e57415b10ca2cec5ac6427bbed8f.1518109128.git.segher@kernel.crashing.org>
From: Segher Boessenkool <segher@kernel.crashing.org>
Date: Thu, 8 Feb 2018 16:58:33 +0000
Subject: [PATCH] 68028

---
 gcc/config/rs6000/rs6000.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 5adccd2..73de617 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4809,25 +4809,6 @@ rs6000_option_override_internal (bool global_init_p)
   if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
     rs6000_print_isa_options (stderr, 0, "after subtarget", rs6000_isa_flags);
 
-  /* For the E500 family of cores, reset the single/double FP flags to let us
-     check that they remain constant across attributes or pragmas.  */
-
-  switch (rs6000_cpu)
-    {
-    case PROCESSOR_PPC8540:
-    case PROCESSOR_PPC8548:
-    case PROCESSOR_PPCE500MC:
-    case PROCESSOR_PPCE500MC64:
-    case PROCESSOR_PPCE5500:
-    case PROCESSOR_PPCE6500:
-      rs6000_single_float = 0;
-      rs6000_double_float = 0;
-      break;
-
-    default:
-      break;
-    }
-
   if (main_target_opt)
     {
       if (main_target_opt->x_rs6000_single_float != rs6000_single_float)
Nick Clifton Feb. 9, 2018, 9:37 a.m. | #2
Hi Segher,


> I thought you were going to do a patch like the following, to make the
> e500 cores less special (they are not):

Sorry - my bad.  I defer to your patch then.  Whatever it takes to get
the BZ fixed and off the books... :-)

Cheers
  Nick
Segher Boessenkool Feb. 10, 2018, 12:14 a.m. | #3
Hi!

On Fri, Feb 09, 2018 at 09:37:28AM +0000, Nick Clifton wrote:
> > I thought you were going to do a patch like the following, to make the
> > e500 cores less special (they are not):
> 
> Sorry - my bad.  I defer to your patch then.  Whatever it takes to get
> the BZ fixed and off the books... :-)

So does it work?  :-)  (I cannot reproduce the problem, there must be
something in your configuration I'm overlooking).


Segher


[ Here it is again, with changelog this time: ]


From 44c3b661ef75e57415b10ca2cec5ac6427bbed8f Mon Sep 17 00:00:00 2001
Message-Id: <44c3b661ef75e57415b10ca2cec5ac6427bbed8f.1518109128.git.segher@kernel.crashing.org>
From: Segher Boessenkool <segher@kernel.crashing.org>
Date: Thu, 8 Feb 2018 16:58:33 +0000
Subject: [PATCH] Do not mess with rs6000_{single,double}_float (PR68028)

For e500 family cores we do some questionable things with those flags,
which does not work with LTO.  So don't.


2018-02-10  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.c (rs6000_option_override_internal): Don't
	handle rs6000_single_float and rs6000_double_float specially for
	e500 family CPUs.

---
 gcc/config/rs6000/rs6000.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 5adccd2..73de617 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4809,25 +4809,6 @@ rs6000_option_override_internal (bool global_init_p)
   if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
     rs6000_print_isa_options (stderr, 0, "after subtarget", rs6000_isa_flags);
 
-  /* For the E500 family of cores, reset the single/double FP flags to let us
-     check that they remain constant across attributes or pragmas.  */
-
-  switch (rs6000_cpu)
-    {
-    case PROCESSOR_PPC8540:
-    case PROCESSOR_PPC8548:
-    case PROCESSOR_PPCE500MC:
-    case PROCESSOR_PPCE500MC64:
-    case PROCESSOR_PPCE5500:
-    case PROCESSOR_PPCE6500:
-      rs6000_single_float = 0;
-      rs6000_double_float = 0;
-      break;
-
-    default:
-      break;
-    }
-
   if (main_target_opt)
     {
       if (main_target_opt->x_rs6000_single_float != rs6000_single_float)
Jeff Law Feb. 17, 2018, 1:05 a.m. | #4
On 02/09/2018 05:14 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Fri, Feb 09, 2018 at 09:37:28AM +0000, Nick Clifton wrote:
>>> I thought you were going to do a patch like the following, to make the
>>> e500 cores less special (they are not):
>>
>> Sorry - my bad.  I defer to your patch then.  Whatever it takes to get
>> the BZ fixed and off the books... :-)
> 
> So does it work?  :-)  (I cannot reproduce the problem, there must be
> something in your configuration I'm overlooking).
> 
> 
> Segher
> 
> 
> [ Here it is again, with changelog this time: ]
> 
> 
> From 44c3b661ef75e57415b10ca2cec5ac6427bbed8f Mon Sep 17 00:00:00 2001
> Message-Id: <44c3b661ef75e57415b10ca2cec5ac6427bbed8f.1518109128.git.segher@kernel.crashing.org>
> From: Segher Boessenkool <segher@kernel.crashing.org>
> Date: Thu, 8 Feb 2018 16:58:33 +0000
> Subject: [PATCH] Do not mess with rs6000_{single,double}_float (PR68028)
> 
> For e500 family cores we do some questionable things with those flags,
> which does not work with LTO.  So don't.
> 
> 
> 2018-02-10  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Don't
> 	handle rs6000_single_float and rs6000_double_float specially for
> 	e500 family CPUs.
That should work -- the whole issue here is the -mcpu=<whatever>
changing other flags.  If your assertion is that we need not treat the
E500 family of cores special here, then let's just zap that code.

Jeff
Segher Boessenkool Feb. 17, 2018, 1:28 a.m. | #5
On Fri, Feb 16, 2018 at 06:05:13PM -0700, Jeff Law wrote:
> > From: Segher Boessenkool <segher@kernel.crashing.org>
> > Date: Thu, 8 Feb 2018 16:58:33 +0000
> > Subject: [PATCH] Do not mess with rs6000_{single,double}_float (PR68028)
> > 
> > For e500 family cores we do some questionable things with those flags,
> > which does not work with LTO.  So don't.
> > 
> > 
> > 2018-02-10  Segher Boessenkool  <segher@kernel.crashing.org>
> > 
> > 	* config/rs6000/rs6000.c (rs6000_option_override_internal): Don't
> > 	handle rs6000_single_float and rs6000_double_float specially for
> > 	e500 family CPUs.

> That should work -- the whole issue here is the -mcpu=<whatever>
> changing other flags.  If your assertion is that we need not treat the
> E500 family of cores special here, then let's just zap that code.

Okay, I'll commit it soon then (I still cannot actually test it, but
yes it is pretty obviously correct; let's hope it solves the original
problem as well).


Segher

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 257282)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -4834,12 +4834,25 @@ 
 
   if (main_target_opt)
     {
-      if (main_target_opt->x_rs6000_single_float != rs6000_single_float)
-	error ("target attribute or pragma changes single precision floating "
-	       "point");
-      if (main_target_opt->x_rs6000_double_float != rs6000_double_float)
-	error ("target attribute or pragma changes double precision floating "
-	       "point");
+      /* PR 68028: In LTO mode the -mcpu value is passed in as a function
+	 attribute rather than on the command line.  So instead of checking
+	 for discrepancioes, we enforce the choice determined by the
+	 attributes.  */
+	 if (in_lto_p)
+	   {
+	     rs6000_single_float = main_target_opt->x_rs6000_single_float;
+	     rs6000_double_float = main_target_opt->x_rs6000_double_float;
+	   }
+	 /* There could be an 'else' statement here but it is hardly worth
+	    it as the compiler will make the optimization anyway, and this
+	    way we avoid indenting the code unnecessarily.  */
+
+	 if (main_target_opt->x_rs6000_single_float != rs6000_single_float)
+	   error ("target attribute or pragma changes single precision floating "
+		      "point");
+	 if (main_target_opt->x_rs6000_double_float != rs6000_double_float)
+	   error ("target attribute or pragma changes double precision floating "
+		  "point");
     }
 
   rs6000_always_hint = (rs6000_tune != PROCESSOR_POWER4