Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-08-05 Thread Segher Boessenkool
On Thu, Aug 05, 2021 at 06:49:21PM +0200, Martin Liška wrote:
> On 8/5/21 5:39 PM, Segher Boessenkool wrote:
> >>>If -mbork is the default, the coompiler whould behave the same if you
> >>>invoke it with -mbork as when you do not.  And the optimize attribute
> >>>should work exactly the same as command line options.
> >>
> >>Ah, got your point. All right, let's use then 'optimize(1)'.
> >>
> >>Is it fine with the adjustment?
> >
> >You are saying the compiler's behaviour is broken, but are changing the
> >testcase to avoid exhibiting that behaviour?
> 
> No, both selections of the 'optimize' attribute trigger the ICE. The reason
> is that any optimize attribute triggers eventually 
> cl_target_option_save/restore
> mechanism which is what the patch addresses.

Aha.  So say that in the test case!  That the test case is to check if
the bug fixed in  has been reintroduced (or describe the bug
more precisely if needed).

Okay for trunk with that fixed.  Thanks!


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-08-05 Thread Martin Liška

On 8/5/21 5:39 PM, Segher Boessenkool wrote:

On Thu, Aug 05, 2021 at 02:05:24PM +0200, Martin Liška wrote:

On 7/23/21 7:57 PM, Segher Boessenkool wrote:

Hi!

On Fri, Jul 23, 2021 at 07:47:54AM +0200, Martin Liška wrote:

On 7/12/21 7:20 PM, Segher Boessenkool wrote:

+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof
(f) *


-fno-stack-protector is default.


Yes, but one needs an optimize attribute in order to trigger
cl_target_option_save/restore
mechanism.


So it behaves differently if you select the default than if you do not
select anything?  That is wrong, no?


Sorry, I don't get your example, please explain it.


If -mbork is the default, the coompiler whould behave the same if you
invoke it with -mbork as when you do not.  And the optimize attribute
should work exactly the same as command line options.


Ah, got your point. All right, let's use then 'optimize(1)'.

Is it fine with the adjustment?


You are saying the compiler's behaviour is broken, but are changing the
testcase to avoid exhibiting that behaviour?


No, both selections of the 'optimize' attribute trigger the ICE. The reason
is that any optimize attribute triggers eventually cl_target_option_save/restore
mechanism which is what the patch addresses.


No, this is not fine.

If a flag is the default the compiler should do the same thing with and
without any attribute setting that flag, directly or indirectly.


Yes, but should not crash. And that's what is my patch dealing with.

Cheers,
Martin




Segher





Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-08-05 Thread Segher Boessenkool
On Thu, Aug 05, 2021 at 02:05:24PM +0200, Martin Liška wrote:
> On 7/23/21 7:57 PM, Segher Boessenkool wrote:
> >Hi!
> >
> >On Fri, Jul 23, 2021 at 07:47:54AM +0200, Martin Liška wrote:
> >>On 7/12/21 7:20 PM, Segher Boessenkool wrote:
> >>+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof
> >>(f) *
> >
> >-fno-stack-protector is default.
> 
> Yes, but one needs an optimize attribute in order to trigger
> cl_target_option_save/restore
> mechanism.
> >>>
> >>>So it behaves differently if you select the default than if you do not
> >>>select anything?  That is wrong, no?
> >>
> >>Sorry, I don't get your example, please explain it.
> >
> >If -mbork is the default, the coompiler whould behave the same if you
> >invoke it with -mbork as when you do not.  And the optimize attribute
> >should work exactly the same as command line options.
> 
> Ah, got your point. All right, let's use then 'optimize(1)'.
> 
> Is it fine with the adjustment?

You are saying the compiler's behaviour is broken, but are changing the
testcase to avoid exhibiting that behaviour?  No, this is not fine.

If a flag is the default the compiler should do the same thing with and
without any attribute setting that flag, directly or indirectly.


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-08-05 Thread Martin Liška

On 7/23/21 7:57 PM, Segher Boessenkool wrote:

Hi!

On Fri, Jul 23, 2021 at 07:47:54AM +0200, Martin Liška wrote:

On 7/12/21 7:20 PM, Segher Boessenkool wrote:

+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof
(f) *


-fno-stack-protector is default.


Yes, but one needs an optimize attribute in order to trigger
cl_target_option_save/restore
mechanism.


So it behaves differently if you select the default than if you do not
select anything?  That is wrong, no?


Sorry, I don't get your example, please explain it.


If -mbork is the default, the coompiler whould behave the same if you
invoke it with -mbork as when you do not.  And the optimize attribute
should work exactly the same as command line options.


Ah, got your point. All right, let's use then 'optimize(1)'.

Is it fine with the adjustment?
Cheers,
Martin



Or perhaps you are saying you have this in the testcase only to exercise
the option save/restore code paths?  Please document that then, in the
testcase.


Segher



>From 517e32a75aa59b4b538eda30e78de0fa925bb0f9 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Tue, 1 Jun 2021 15:39:14 +0200
Subject: [PATCH] rs6000: Fix restored rs6000_long_double_type_size

As mentioned in the "Fallout: save/restore target options in handle_optimize_attribute"
thread, we need to support target option restore
of rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.

gcc/ChangeLog:

	* config/rs6000/rs6000.c (rs6000_option_override_internal): When
	a target option is restored, it can have
	rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode
	and error should not be emitted.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pragma-optimize.c: New test.
---
 gcc/config/rs6000/rs6000.c |  2 ++
 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c | 13 +
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2de5a96e1b6..5b1c06b09fc 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4189,6 +4189,8 @@ rs6000_option_override_internal (bool global_init_p)
   else
 	rs6000_long_double_type_size = default_long_double_size;
 }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option value can be seen when cl_target_option_restore is called.  */
   else if (rs6000_long_double_type_size == 128)
 rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
   else if (global_options_set.x_rs6000_ieeequad)
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
new file mode 100644
index 000..e8ba63a0667
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,13 @@
+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize (1))) __typeof (f) *
+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}
-- 
2.32.0



Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size

2021-07-23 Thread Segher Boessenkool
Hi!

On Tue, Jun 01, 2021 at 03:39:14PM +0200, Martin Liska wrote:
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): When
>   a target option is restored, it can have
>   rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode
>   and error should not be emitted.

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4186,6 +4186,8 @@ rs6000_option_override_internal (bool global_init_p)
>else
>   rs6000_long_double_type_size = default_long_double_size;
>  }
> +  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
> +; /* The option value can be seen when cl_target_option_restore is 
> called.  */

(line too long)

The comment is still very cryptic.  What you have in the changelog is
much better digestible.

Okay for trunk with those things fixed.  Thanks!


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-07-23 Thread Segher Boessenkool
Hi!

On Fri, Jul 23, 2021 at 07:47:54AM +0200, Martin Liška wrote:
> On 7/12/21 7:20 PM, Segher Boessenkool wrote:
> +static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof 
> (f) *
> >>>
> >>>-fno-stack-protector is default.
> >>
> >>Yes, but one needs an optimize attribute in order to trigger
> >>cl_target_option_save/restore
> >>mechanism.
> >
> >So it behaves differently if you select the default than if you do not
> >select anything?  That is wrong, no?
> 
> Sorry, I don't get your example, please explain it.

If -mbork is the default, the coompiler whould behave the same if you
invoke it with -mbork as when you do not.  And the optimize attribute
should work exactly the same as command line options.

Or perhaps you are saying you have this in the testcase only to exercise
the option save/restore code paths?  Please document that then, in the
testcase.


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-07-22 Thread Martin Liška

On 7/12/21 7:20 PM, Segher Boessenkool wrote:

On Mon, Jun 28, 2021 at 02:19:03PM +0200, Martin Liška wrote:

On 6/24/21 12:46 AM, Segher Boessenkool wrote:

As mentioned in the "Fallout: save/restore target options in
handle_optimize_attribute"
thread, we need to support target option restore of
rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.


I have no idea?  Could you explain please?


Sure. Few weeks ago, we started using cl_target_option_{save,restore} calls
even for optimize attributes (and pragma). Motivation was that optimize
options
can influence target options (and vice versa).

Doing that, FLOAT_PRECISION_TFmode must be accepted as a valid option value
for rs6000_long_double_type_size.




--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */


Why on Linux only?  That doesn't sound right.  Do you need some other
selector(s)?


Sorry, I copied the test-case.


Ugh.  Yes, the status quo is no good either :-(


+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *


-fno-stack-protector is default.


Yes, but one needs an optimize attribute in order to trigger
cl_target_option_save/restore
mechanism.


So it behaves differently if you select the default than if you do not
select anything?  That is wrong, no?


Sorry, I don't get your example, please explain it.




>From 1632939853fbf193f72ace3d1024a137d549fef4 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Tue, 1 Jun 2021 15:39:14 +0200
Subject: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.


(No full stop at end of subject please)


Done.



Missing patch description here.  This should be suitable as commit
message when you eventually commit the patch.

Please send with that, as a separate mail, not as attachment to another
thread.


Done.




gcc/ChangeLog:

* config/rs6000/rs6000.c (rs6000_option_override_internal): When
a target option is restored, it can have
rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode.


That does not say what changed?


Updated.




--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
else
rs6000_long_double_type_size = default_long_double_size;
  }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option can be restored with cl_target_option_restore.  */
else if (rs6000_long_double_type_size == 128)
  rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
else if (global_options_set.x_rs6000_ieeequad)


"The option can be restored" is more confusing than helpful.  *Will* be
restored by it, maybe?  Not that I understand what that means :-/


I updated the wording a bit.



Does it make more sense to merge the 128 and FLOAT_PRECISION_TFmode
cases?


Likely not, it's important to assign a reasonable comment to the 128 option 
value.




diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c 
b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
new file mode 100644
index 000..2455fb57138
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */


No target powerpc*-*-* in gcc.target/powerpc please.  This is enforced
for everything in there by powerpc.exp already.


Fixed.

Martin



Thanks,


Segher





[PATCH] rs6000: Fix restored rs6000_long_double_type_size

2021-07-22 Thread Martin Liska
As mentioned in the "Fallout: save/restore target options in 
handle_optimize_attribute"
thread, we need to support target option restore
of rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.

gcc/ChangeLog:

* config/rs6000/rs6000.c (rs6000_option_override_internal): When
a target option is restored, it can have
rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode
and error should not be emitted.

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/pragma-optimize.c: New test.
---
 gcc/config/rs6000/rs6000.c |  2 ++
 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c | 13 +
 2 files changed, 15 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 279f00cc648..510936a9948 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4186,6 +4186,8 @@ rs6000_option_override_internal (bool global_init_p)
   else
rs6000_long_double_type_size = default_long_double_size;
 }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option value can be seen when cl_target_option_restore is called. 
 */
   else if (rs6000_long_double_type_size == 128)
 rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
   else if (global_options_set.x_rs6000_ieeequad)
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c 
b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
new file mode 100644
index 000..4a86b58f27c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,13 @@
+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}
-- 
2.32.0



Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-07-12 Thread Segher Boessenkool
On Mon, Jun 28, 2021 at 02:19:03PM +0200, Martin Liška wrote:
> On 6/24/21 12:46 AM, Segher Boessenkool wrote:
> >>As mentioned in the "Fallout: save/restore target options in
> >>handle_optimize_attribute"
> >>thread, we need to support target option restore of
> >>rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.
> >
> >I have no idea?  Could you explain please?
> 
> Sure. Few weeks ago, we started using cl_target_option_{save,restore} calls
> even for optimize attributes (and pragma). Motivation was that optimize 
> options
> can influence target options (and vice versa).
> 
> Doing that, FLOAT_PRECISION_TFmode must be accepted as a valid option value
> for rs6000_long_double_type_size.


> >>--- /dev/null
> >>+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
> >>@@ -0,0 +1,14 @@
> >>+/* { dg-do compile { target { powerpc*-*-linux* } } } */
> >
> >Why on Linux only?  That doesn't sound right.  Do you need some other
> >selector(s)?
> 
> Sorry, I copied the test-case.

Ugh.  Yes, the status quo is no good either :-(

> >>+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
> >>+
> >>+extern unsigned long int x;
> >>+extern float f (float);
> >>+extern __typeof (f) f_power8;
> >>+extern __typeof (f) f_power9;
> >>+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
> >>+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
> >
> >-fno-stack-protector is default.
> 
> Yes, but one needs an optimize attribute in order to trigger 
> cl_target_option_save/restore
> mechanism.

So it behaves differently if you select the default than if you do not
select anything?  That is wrong, no?

> >From 1632939853fbf193f72ace3d1024a137d549fef4 Mon Sep 17 00:00:00 2001
> From: Martin Liska 
> Date: Tue, 1 Jun 2021 15:39:14 +0200
> Subject: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

(No full stop at end of subject please)

Missing patch description here.  This should be suitable as commit
message when you eventually commit the patch.

Please send with that, as a separate mail, not as attachment to another
thread.

> gcc/ChangeLog:
> 
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): When
>   a target option is restored, it can have
>   rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode.

That does not say what changed?

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
>else
>   rs6000_long_double_type_size = default_long_double_size;
>  }
> +  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
> +; /* The option can be restored with cl_target_option_restore.  */
>else if (rs6000_long_double_type_size == 128)
>  rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
>else if (global_options_set.x_rs6000_ieeequad)

"The option can be restored" is more confusing than helpful.  *Will* be
restored by it, maybe?  Not that I understand what that means :-/

Does it make more sense to merge the 128 and FLOAT_PRECISION_TFmode
cases?

> diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c 
> b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
> new file mode 100644
> index 000..2455fb57138
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */

No target powerpc*-*-* in gcc.target/powerpc please.  This is enforced
for everything in there by powerpc.exp already.

Thanks,


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-07-12 Thread Segher Boessenkool
On Mon, Jul 12, 2021 at 06:19:28AM +0200, Martin Liška wrote:
> PING^1

I did not notice you attached a new patch.  It works a lot better if
every patch series is a new thread.


Segher


Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-07-11 Thread Martin Liška

PING^1

On 6/28/21 2:19 PM, Martin Liška wrote:

On 6/24/21 12:46 AM, Segher Boessenkool wrote:

Hi!

On Wed, Jun 23, 2021 at 03:22:34PM +0200, Martin Liška wrote:

As mentioned in the "Fallout: save/restore target options in
handle_optimize_attribute"
thread, we need to support target option restore of
rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.


I have no idea?  Could you explain please?


Sure. Few weeks ago, we started using cl_target_option_{save,restore} calls
even for optimize attributes (and pragma). Motivation was that optimize options
can influence target options (and vice versa).

Doing that, FLOAT_PRECISION_TFmode must be accepted as a valid option value
for rs6000_long_double_type_size.




--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
    else
  rs6000_long_double_type_size = default_long_double_size;
  }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+    ; /* The option can be restored a TREE_TARGET_OPTION.  */


What does that mean?  It is not grammatical, and not obvious what it
should mean.


Updated.




--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */


Why on Linux only?  That doesn't sound right.  Do you need some other
selector(s)?


Sorry, I copied the test-case.




+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *


-fno-stack-protector is default.


Yes, but one needs an optimize attribute in order to trigger 
cl_target_option_save/restore
mechanism.

Martin




+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}


The testcase should say what it is testing for, it is not obvious?


Segher







Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-06-28 Thread Martin Liška

On 6/24/21 12:46 AM, Segher Boessenkool wrote:

Hi!

On Wed, Jun 23, 2021 at 03:22:34PM +0200, Martin Liška wrote:

As mentioned in the "Fallout: save/restore target options in
handle_optimize_attribute"
thread, we need to support target option restore of
rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.


I have no idea?  Could you explain please?


Sure. Few weeks ago, we started using cl_target_option_{save,restore} calls
even for optimize attributes (and pragma). Motivation was that optimize options
can influence target options (and vice versa).

Doing that, FLOAT_PRECISION_TFmode must be accepted as a valid option value
for rs6000_long_double_type_size.




--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
else
rs6000_long_double_type_size = default_long_double_size;
  }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option can be restored a TREE_TARGET_OPTION.  */


What does that mean?  It is not grammatical, and not obvious what it
should mean.


Updated.




--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */


Why on Linux only?  That doesn't sound right.  Do you need some other
selector(s)?


Sorry, I copied the test-case.




+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *


-fno-stack-protector is default.


Yes, but one needs an optimize attribute in order to trigger 
cl_target_option_save/restore
mechanism.

Martin




+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}


The testcase should say what it is testing for, it is not obvious?


Segher



>From 1632939853fbf193f72ace3d1024a137d549fef4 Mon Sep 17 00:00:00 2001
From: Martin Liska 
Date: Tue, 1 Jun 2021 15:39:14 +0200
Subject: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

gcc/ChangeLog:

	* config/rs6000/rs6000.c (rs6000_option_override_internal): When
	a target option is restored, it can have
	rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode.

gcc/testsuite/ChangeLog:

	* gcc.target/powerpc/pragma-optimize.c: New test.
---
 gcc/config/rs6000/rs6000.c |  2 ++
 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c | 14 ++
 2 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2c249e186e1..fa4aa864c00 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
   else
 	rs6000_long_double_type_size = default_long_double_size;
 }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option can be restored with cl_target_option_restore.  */
   else if (rs6000_long_double_type_size == 128)
 rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
   else if (global_options_set.x_rs6000_ieeequad)
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
new file mode 100644
index 000..2455fb57138
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}
-- 
2.32.0



Re: [PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-06-23 Thread Segher Boessenkool
Hi!

On Wed, Jun 23, 2021 at 03:22:34PM +0200, Martin Liška wrote:
> As mentioned in the "Fallout: save/restore target options in 
> handle_optimize_attribute"
> thread, we need to support target option restore of 
> rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.

I have no idea?  Could you explain please?

> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
>else
>   rs6000_long_double_type_size = default_long_double_size;
>  }
> +  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
> +; /* The option can be restored a TREE_TARGET_OPTION.  */

What does that mean?  It is not grammatical, and not obvious what it
should mean.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { powerpc*-*-linux* } } } */

Why on Linux only?  That doesn't sound right.  Do you need some other
selector(s)?

> +/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
> +
> +extern unsigned long int x;
> +extern float f (float);
> +extern __typeof (f) f_power8;
> +extern __typeof (f) f_power9;
> +extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
> +static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *

-fno-stack-protector is default.

> +f_ifunc (void)
> +{
> +  __typeof (f) *res = x ? f_power9 : f_power8;
> +  return res;
> +}

The testcase should say what it is testing for, it is not obvious?


Segher


[PATCH] rs6000: Fix restored rs6000_long_double_type_size.

2021-06-23 Thread Martin Liška

Hello.

As mentioned in the "Fallout: save/restore target options in 
handle_optimize_attribute"
thread, we need to support target option restore of 
rs6000_long_double_type_size == FLOAT_PRECISION_TFmode.

Patch can bootstrap on ppc64le-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

* config/rs6000/rs6000.c (rs6000_option_override_internal): When
a target option is restored, it can have
rs6000_long_double_type_size set to FLOAT_PRECISION_TFmode.

gcc/testsuite/ChangeLog:

* gcc.target/powerpc/pragma-optimize.c: New test.
---
 gcc/config/rs6000/rs6000.c |  2 ++
 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c | 14 ++
 2 files changed, 16 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pragma-optimize.c

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 835af7708f9..2c811480db9 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4185,6 +4185,8 @@ rs6000_option_override_internal (bool global_init_p)
   else
rs6000_long_double_type_size = default_long_double_size;
 }
+  else if (rs6000_long_double_type_size == FLOAT_PRECISION_TFmode)
+; /* The option can be restored a TREE_TARGET_OPTION.  */
   else if (rs6000_long_double_type_size == 128)
 rs6000_long_double_type_size = FLOAT_PRECISION_TFmode;
   else if (global_options_set.x_rs6000_ieeequad)
diff --git a/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c 
b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
new file mode 100644
index 000..629bfcee0ec
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pragma-optimize.c
@@ -0,0 +1,14 @@
+/* { dg-do compile { target { powerpc*-*-linux* } } } */
+/* { dg-options "-O2 -mlong-double-128 -mabi=ibmlongdouble" } */
+
+extern unsigned long int x;
+extern float f (float);
+extern __typeof (f) f_power8;
+extern __typeof (f) f_power9;
+extern __typeof (f) f __attribute__ ((ifunc ("f_ifunc")));
+static __attribute__ ((optimize ("-fno-stack-protector"))) __typeof (f) *
+f_ifunc (void)
+{
+  __typeof (f) *res = x ? f_power9 : f_power8;
+  return res;
+}
--
2.32.0