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 <mli...@suse.cz>
> 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 00000000000..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
