Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
options-save.c: In function 'void cl_target_option_save(cl_target_option*, gcc_options*, gcc_options*)': options-save.c:8526:26: error: unused variable 'mask' [-Werror=unused-variable] 8526 | unsigned HOST_WIDE_INT mask = 0; | ^~~~ options-save.c: In function 'void cl_target_option_restore(gcc_options*, gcc_options*, cl_target_option*)': options-save.c:8537:26: error: unused variable 'mask' [-Werror=unused-variable] 8537 | unsigned HOST_WIDE_INT mask; | ^~~~ Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On October 5, 2020 9:08:41 AM GMT+02:00, Jakub Jelinek wrote:
>On Sun, Oct 04, 2020 at 09:16:00PM +0200, Jakub Jelinek via Gcc-patches
>wrote:
>> On Sun, Oct 04, 2020 at 09:13:29AM +0200, Andreas Schwab wrote:
>> > This breaks ia64:
>> >
>> > In file included from ./tm.h:23,
>> > from ../../gcc/gencheck.c:23:
>> > ./options.h:7816:40: error: ISO C++ forbids zero-size array
>'explicit_mask' [-Werror=pedantic]
>> > 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
>> > |^
>> > ./options.h:7816:26: error: zero-size array member
>'cl_target_option::explicit_mask' not at end of 'struct
>cl_target_option' [-Werror=pedantic]
>> > 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
>> > | ^
>> > ./options.h:7812:16: note: in the definition of 'struct
>cl_target_option'
>> > 7812 | struct GTY(()) cl_target_option
>> > |^~~~
>>
>> Oops, sorry.
>>
>> The following patch should fix that and should also fix streaming of
>the
>> new explicit_mask_* members.
>> I'll bootstrap/regtest on x86_64-linux and i686-linux tonight, but
>have no
>> way to test it on ia64-linux (well, tested that x86_64-linux ->
>ia64-linux
>> cross builds cc1 with it).
>
>Successfully bootstrapped/regtested on x86_64-linux and i686-linux, ok
>for
>trunk?
OK.
Richard.
>> 2020-10-04 Jakub Jelinek
>>
>> * opth-gen.awk: Don't emit explicit_mask array if n_target_explicit
>> is equal to n_target_explicit_mask.
>> * optc-save-gen.awk: Compute has_target_explicit_mask and if false,
>> don't emit code iterating over explicit_mask array elements. Stream
>> also explicit_mask_* target members.
>>
>> --- gcc/opth-gen.awk.jj 2020-10-03 21:21:59.727862692 +0200
>> +++ gcc/opth-gen.awk 2020-10-04 11:12:51.851906413 +0200
>> @@ -291,7 +291,10 @@ for (i = 0; i < n_target_char; i++) {
>> }
>>
>> print " /* " n_target_explicit - n_target_explicit_mask " members
>*/";
>> -print " unsigned HOST_WIDE_INT explicit_mask[" int
>((n_target_explicit - n_target_explicit_mask + 63) / 64) "];";
>> +if (n_target_explicit > n_target_explicit_mask) {
>> +print " unsigned HOST_WIDE_INT explicit_mask[" \
>> + int ((n_target_explicit - n_target_explicit_mask + 63) / 64)
>"];";
>> +}
>>
>> for (i = 0; i < n_target_explicit_mask; i++) {
>> print " " var_target_explicit_mask[i] ";";
>> --- gcc/optc-save-gen.awk.jj 2020-10-03 21:21:59.728862678 +0200
>> +++ gcc/optc-save-gen.awk2020-10-04 21:03:31.619462434 +0200
>> @@ -689,6 +689,10 @@ for (i = 0; i < n_target_string; i++) {
>> if (j != 0) {
>> print " ptr->explicit_mask[" k "] = mask;";
>> }
>> +has_target_explicit_mask = 0;
>> +if (j != 0 || k != 0) {
>> +has_target_explicit_mask = 1;
>> +}
>>
>> print "}";
>>
>> @@ -1075,9 +1079,11 @@ for (i = 0; i < n_target_val; i++) {
>> print "return false;";
>> }
>>
>> -print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) /
>sizeof (ptr1->explicit_mask[0]); i++)";
>> -print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
>> -print " return false;"
>> +if (has_target_explicit_mask) {
>> +print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) /
>sizeof (ptr1->explicit_mask[0]); i++)";
>> +print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
>> +print " return false;"
>> +}
>>
>> for (i = 0; i < n_target_other; i++) {
>> if (var_target_other[i] in var_target_explicit_mask) {
>> @@ -1121,8 +1127,10 @@ for (i = 0; i < n_target_val; i++) {
>> name = var_target_val[i]
>> print " hstate.add_hwi (ptr->" name");";
>> }
>> -print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
>(ptr->explicit_mask[0]); i++)";
>> -print "hstate.add_hwi (ptr->explicit_mask[i]);";
>> +if (has_target_explicit_mask) {
>> +print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) /
>sizeof (ptr->explicit_mask[0]); i++)";
>> +print "hstate.add_hwi (ptr->explicit_mask[i]);";
>> +}
>>
>> for (i = 0; i < n_target_other; i++) {
>> if (var_target_other[i] in var_target_explicit_mask)
>> @@ -1159,8 +1167,22 @@ for (i = 0; i < n_target_val; i++) {
>> print " bp_pack_value (bp, ptr->" name", 64);";
>> }
>>
>> -print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
>(ptr->explicit_mask[0]); i++)";
>> -print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
>> +if (has_target_explicit_mask) {
>> +print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) /
>sizeof (ptr->explicit_mask[0]); i++)";
>> +print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
>> +}
>> +
>> +for (i = 0; i < n_target_other; i++) {
>> +if (var_target_other[i] in var_target_explicit_mask) {
>> +print " bp_pack_value (bp, ptr->explicit_mask_"
>var_target_other[i] ", 64);";
>> +}
>> +}
>> +
>> +for (i = 0; i < n_target_int; i++) {
>> +if
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Sun, Oct 04, 2020 at 09:16:00PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Sun, Oct 04, 2020 at 09:13:29AM +0200, Andreas Schwab wrote:
> > This breaks ia64:
> >
> > In file included from ./tm.h:23,
> > from ../../gcc/gencheck.c:23:
> > ./options.h:7816:40: error: ISO C++ forbids zero-size array 'explicit_mask'
> > [-Werror=pedantic]
> > 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
> > |^
> > ./options.h:7816:26: error: zero-size array member
> > 'cl_target_option::explicit_mask' not at end of 'struct cl_target_option'
> > [-Werror=pedantic]
> > 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
> > | ^
> > ./options.h:7812:16: note: in the definition of 'struct cl_target_option'
> > 7812 | struct GTY(()) cl_target_option
> > |^~~~
>
> Oops, sorry.
>
> The following patch should fix that and should also fix streaming of the
> new explicit_mask_* members.
> I'll bootstrap/regtest on x86_64-linux and i686-linux tonight, but have no
> way to test it on ia64-linux (well, tested that x86_64-linux -> ia64-linux
> cross builds cc1 with it).
Successfully bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
> 2020-10-04 Jakub Jelinek
>
> * opth-gen.awk: Don't emit explicit_mask array if n_target_explicit
> is equal to n_target_explicit_mask.
> * optc-save-gen.awk: Compute has_target_explicit_mask and if false,
> don't emit code iterating over explicit_mask array elements. Stream
> also explicit_mask_* target members.
>
> --- gcc/opth-gen.awk.jj 2020-10-03 21:21:59.727862692 +0200
> +++ gcc/opth-gen.awk 2020-10-04 11:12:51.851906413 +0200
> @@ -291,7 +291,10 @@ for (i = 0; i < n_target_char; i++) {
> }
>
> print " /* " n_target_explicit - n_target_explicit_mask " members */";
> -print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit -
> n_target_explicit_mask + 63) / 64) "];";
> +if (n_target_explicit > n_target_explicit_mask) {
> + print " unsigned HOST_WIDE_INT explicit_mask[" \
> + int ((n_target_explicit - n_target_explicit_mask + 63) / 64) "];";
> +}
>
> for (i = 0; i < n_target_explicit_mask; i++) {
> print " " var_target_explicit_mask[i] ";";
> --- gcc/optc-save-gen.awk.jj 2020-10-03 21:21:59.728862678 +0200
> +++ gcc/optc-save-gen.awk 2020-10-04 21:03:31.619462434 +0200
> @@ -689,6 +689,10 @@ for (i = 0; i < n_target_string; i++) {
> if (j != 0) {
> print " ptr->explicit_mask[" k "] = mask;";
> }
> +has_target_explicit_mask = 0;
> +if (j != 0 || k != 0) {
> + has_target_explicit_mask = 1;
> +}
>
> print "}";
>
> @@ -1075,9 +1079,11 @@ for (i = 0; i < n_target_val; i++) {
> print "return false;";
> }
>
> -print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) / sizeof
> (ptr1->explicit_mask[0]); i++)";
> -print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
> -print " return false;"
> +if (has_target_explicit_mask) {
> + print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) / sizeof
> (ptr1->explicit_mask[0]); i++)";
> + print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
> + print " return false;"
> +}
>
> for (i = 0; i < n_target_other; i++) {
> if (var_target_other[i] in var_target_explicit_mask) {
> @@ -1121,8 +1127,10 @@ for (i = 0; i < n_target_val; i++) {
> name = var_target_val[i]
> print " hstate.add_hwi (ptr->" name");";
> }
> -print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
> (ptr->explicit_mask[0]); i++)";
> -print "hstate.add_hwi (ptr->explicit_mask[i]);";
> +if (has_target_explicit_mask) {
> + print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
> (ptr->explicit_mask[0]); i++)";
> + print "hstate.add_hwi (ptr->explicit_mask[i]);";
> +}
>
> for (i = 0; i < n_target_other; i++) {
> if (var_target_other[i] in var_target_explicit_mask)
> @@ -1159,8 +1167,22 @@ for (i = 0; i < n_target_val; i++) {
> print " bp_pack_value (bp, ptr->" name", 64);";
> }
>
> -print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
> (ptr->explicit_mask[0]); i++)";
> -print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
> +if (has_target_explicit_mask) {
> + print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
> (ptr->explicit_mask[0]); i++)";
> + print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
> +}
> +
> +for (i = 0; i < n_target_other; i++) {
> + if (var_target_other[i] in var_target_explicit_mask) {
> + print " bp_pack_value (bp, ptr->explicit_mask_"
> var_target_other[i] ", 64);";
> + }
> +}
> +
> +for (i = 0; i < n_target_int; i++) {
> + if (var_target_int[i] in var_target_explicit_mask) {
> + print " bp_pack_value (bp, ptr->explicit_mask_"
> var_target_int[i] ",
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Sun, Oct 04, 2020 at 09:13:29AM +0200, Andreas Schwab wrote:
> This breaks ia64:
>
> In file included from ./tm.h:23,
> from ../../gcc/gencheck.c:23:
> ./options.h:7816:40: error: ISO C++ forbids zero-size array 'explicit_mask'
> [-Werror=pedantic]
> 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
> |^
> ./options.h:7816:26: error: zero-size array member
> 'cl_target_option::explicit_mask' not at end of 'struct cl_target_option'
> [-Werror=pedantic]
> 7816 | unsigned HOST_WIDE_INT explicit_mask[0];
> | ^
> ./options.h:7812:16: note: in the definition of 'struct cl_target_option'
> 7812 | struct GTY(()) cl_target_option
> |^~~~
Oops, sorry.
The following patch should fix that and should also fix streaming of the
new explicit_mask_* members.
I'll bootstrap/regtest on x86_64-linux and i686-linux tonight, but have no
way to test it on ia64-linux (well, tested that x86_64-linux -> ia64-linux
cross builds cc1 with it).
2020-10-04 Jakub Jelinek
* opth-gen.awk: Don't emit explicit_mask array if n_target_explicit
is equal to n_target_explicit_mask.
* optc-save-gen.awk: Compute has_target_explicit_mask and if false,
don't emit code iterating over explicit_mask array elements. Stream
also explicit_mask_* target members.
--- gcc/opth-gen.awk.jj 2020-10-03 21:21:59.727862692 +0200
+++ gcc/opth-gen.awk2020-10-04 11:12:51.851906413 +0200
@@ -291,7 +291,10 @@ for (i = 0; i < n_target_char; i++) {
}
print " /* " n_target_explicit - n_target_explicit_mask " members */";
-print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit -
n_target_explicit_mask + 63) / 64) "];";
+if (n_target_explicit > n_target_explicit_mask) {
+ print " unsigned HOST_WIDE_INT explicit_mask[" \
+ int ((n_target_explicit - n_target_explicit_mask + 63) / 64) "];";
+}
for (i = 0; i < n_target_explicit_mask; i++) {
print " " var_target_explicit_mask[i] ";";
--- gcc/optc-save-gen.awk.jj2020-10-03 21:21:59.728862678 +0200
+++ gcc/optc-save-gen.awk 2020-10-04 21:03:31.619462434 +0200
@@ -689,6 +689,10 @@ for (i = 0; i < n_target_string; i++) {
if (j != 0) {
print " ptr->explicit_mask[" k "] = mask;";
}
+has_target_explicit_mask = 0;
+if (j != 0 || k != 0) {
+ has_target_explicit_mask = 1;
+}
print "}";
@@ -1075,9 +1079,11 @@ for (i = 0; i < n_target_val; i++) {
print "return false;";
}
-print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) / sizeof
(ptr1->explicit_mask[0]); i++)";
-print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
-print " return false;"
+if (has_target_explicit_mask) {
+ print " for (size_t i = 0; i < sizeof (ptr1->explicit_mask) / sizeof
(ptr1->explicit_mask[0]); i++)";
+ print "if (ptr1->explicit_mask[i] != ptr2->explicit_mask[i])";
+ print " return false;"
+}
for (i = 0; i < n_target_other; i++) {
if (var_target_other[i] in var_target_explicit_mask) {
@@ -1121,8 +1127,10 @@ for (i = 0; i < n_target_val; i++) {
name = var_target_val[i]
print " hstate.add_hwi (ptr->" name");";
}
-print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
(ptr->explicit_mask[0]); i++)";
-print "hstate.add_hwi (ptr->explicit_mask[i]);";
+if (has_target_explicit_mask) {
+ print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
(ptr->explicit_mask[0]); i++)";
+ print "hstate.add_hwi (ptr->explicit_mask[i]);";
+}
for (i = 0; i < n_target_other; i++) {
if (var_target_other[i] in var_target_explicit_mask)
@@ -1159,8 +1167,22 @@ for (i = 0; i < n_target_val; i++) {
print " bp_pack_value (bp, ptr->" name", 64);";
}
-print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
(ptr->explicit_mask[0]); i++)";
-print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
+if (has_target_explicit_mask) {
+ print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
(ptr->explicit_mask[0]); i++)";
+ print "bp_pack_value (bp, ptr->explicit_mask[i], 64);";
+}
+
+for (i = 0; i < n_target_other; i++) {
+ if (var_target_other[i] in var_target_explicit_mask) {
+ print " bp_pack_value (bp, ptr->explicit_mask_"
var_target_other[i] ", 64);";
+ }
+}
+
+for (i = 0; i < n_target_int; i++) {
+ if (var_target_int[i] in var_target_explicit_mask) {
+ print " bp_pack_value (bp, ptr->explicit_mask_"
var_target_int[i] ", 64);";
+ }
+}
print "}";
@@ -1188,8 +1210,22 @@ for (i = 0; i < n_target_val; i++) {
print " ptr->" name" = (" var_target_val_type[i] ") bp_unpack_value
(bp, 64);";
}
-print " for (size_t i = 0; i < sizeof (ptr->explicit_mask) / sizeof
(ptr->explicit_mask[0]); i++)";
-print "ptr->explicit_mask[i] =
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
This breaks ia64:
In file included from ./tm.h:23,
from ../../gcc/gencheck.c:23:
./options.h:7816:40: error: ISO C++ forbids zero-size array 'explicit_mask'
[-Werror=pedantic]
7816 | unsigned HOST_WIDE_INT explicit_mask[0];
|^
./options.h:7816:26: error: zero-size array member
'cl_target_option::explicit_mask' not at end of 'struct cl_target_option'
[-Werror=pedantic]
7816 | unsigned HOST_WIDE_INT explicit_mask[0];
| ^
./options.h:7812:16: note: in the definition of 'struct cl_target_option'
7812 | struct GTY(()) cl_target_option
|^~~~
$ diff -u gcc-2020100[34]/Build/gcc/options.h
--- gcc-20201003/Build/gcc/options.h2020-10-03 04:50:58.0 +0200
+++ gcc-20201004/Build/gcc/options.h2020-10-04 04:25:18.0 +0200
@@ -7812,8 +7812,9 @@
struct GTY(()) cl_target_option
{
int x_target_flags;
- /* 1 members */
- unsigned HOST_WIDE_INT explicit_mask[1];
+ /* 0 members */
+ unsigned HOST_WIDE_INT explicit_mask[0];
+ int explicit_mask_target_flags;
};
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On October 3, 2020 10:41:26 AM GMT+02:00, Jakub Jelinek
wrote:
>On Fri, Oct 02, 2020 at 04:21:12PM +0200, Stefan Schulze Frielinghaus
>via Gcc-patches wrote:
>> > > Sure, no problem at all. In that case I stop to investigate
>further and
>> > > wait for you.
>> >
>> > Here is a patch that implements that.
>> >
>> > Can you please check if it fixes the s390x regressions that I
>couldn't
>> > reproduce in a cross?
>>
>> Bootstrapped and regtested on S/390. Now all tattr-*.c test cases run
>> successfully with the patch. All other tests remain the same.
>
>I've now also successfully tested it on
>{x86_64,i686,armv7hl,aarch64,powerpc64le,s390x. Ok for trunk?
OK.
Richard.
>> > 2020-10-02 Jakub Jelinek
>> >
>> >* opth-gen.awk: For variables referenced in Mask and InverseMask,
>> >don't use the explicit_mask bitmask array, but add separate
>> >explicit_mask_* members with the same types as the variables.
>> >* optc-save-gen.awk: Save, restore, compare and hash the separate
>> >explicit_mask_* members.
>> >
>> > --- gcc/opth-gen.awk.jj2020-09-14 09:04:35.866854351 +0200
>> > +++ gcc/opth-gen.awk 2020-10-01 21:52:30.855122749 +0200
>> > @@ -209,6 +209,7 @@ n_target_int = 0;
>> > n_target_enum = 0;
>> > n_target_other = 0;
>> > n_target_explicit = n_extra_target_vars;
>> > +n_target_explicit_mask = 0;
>> >
>> > for (i = 0; i < n_target_save; i++) {
>> >if (target_save_decl[i] ~ "^((un)?signed +)?int +[_" alnum "]+$")
>> > @@ -240,6 +241,12 @@ if (have_save) {
>> >var_save_seen[name]++;
>> >n_target_explicit++;
>> >otype = var_type_struct(flags[i])
>> > +
>> > + if (opt_args("Mask", flags[i]) != "" \
>> > + || opt_args("InverseMask", flags[i]))
>> > +
>> > var_target_explicit_mask[n_target_explicit_mask++] \
>> > + = otype "explicit_mask_" name;
>> > +
>> >if (otype ~ "^((un)?signed +)?int *$")
>> >var_target_int[n_target_int++] = otype "x_"
>> > name;
>> >
>> > @@ -259,6 +266,8 @@ if (have_save) {
>> > } else {
>> >var_target_int[n_target_int++] = "int x_target_flags";
>> >n_target_explicit++;
>> > + var_target_explicit_mask[n_target_explicit_mask++] \
>> > + = "int explicit_mask_target_flags";
>> > }
>> >
>> > for (i = 0; i < n_target_other; i++) {
>> > @@ -281,8 +290,12 @@ for (i = 0; i < n_target_char; i++) {
>> >print " " var_target_char[i] ";";
>> > }
>> >
>> > -print " /* " n_target_explicit " members */";
>> > -print " unsigned HOST_WIDE_INT explicit_mask[" int
>((n_target_explicit + 63) / 64) "];";
>> > +print " /* " n_target_explicit - n_target_explicit_mask " members
>*/";
>> > +print " unsigned HOST_WIDE_INT explicit_mask[" int
>((n_target_explicit - n_target_explicit_mask + 63) / 64) "];";
>> > +
>> > +for (i = 0; i < n_target_explicit_mask; i++) {
>> > + print " " var_target_explicit_mask[i] ";";
>> > +}
>> >
>> > print "};";
>> > print "";
>> > --- gcc/optc-save-gen.awk.jj 2020-09-16 10:06:23.018093486 +0200
>> > +++ gcc/optc-save-gen.awk 2020-10-01 21:48:10.933868862 +0200
>> > @@ -516,6 +516,10 @@ if (have_save) {
>> >
>> >var_save_seen[name]++;
>> >otype = var_type_struct(flags[i])
>> > + if (opt_args("Mask", flags[i]) != "" \
>> > + || opt_args("InverseMask", flags[i]))
>> > + var_target_explicit_mask[name] = 1;
>> > +
>> >if (otype ~ "^((un)?signed +)?int *$")
>> >var_target_int[n_target_int++] = name;
>> >
>> > @@ -545,6 +549,7 @@ if (have_save) {
>> >}
>> > } else {
>> >var_target_int[n_target_int++] = "target_flags";
>> > + var_target_explicit_mask["target_flags"] = 1;
>> > }
>> >
>> > have_assert = 0;
>> > @@ -608,6 +613,10 @@ for (i = 0; i < n_extra_target_vars; i++
>> > }
>> >
>> > for (i = 0; i < n_target_other; i++) {
>> > + if (var_target_other[i] in var_target_explicit_mask) {
>> > + print " ptr->explicit_mask_" var_target_other[i] " =
>opts_set->x_" var_target_other[i] ";";
>> > + continue;
>> > + }
>> >print " if (opts_set->x_" var_target_other[i] ") mask |=
>HOST_WIDE_INT_1U << " j ";";
>> >j++;
>> >if (j == 64) {
>> > @@ -630,6 +639,10 @@ for (i = 0; i < n_target_enum; i++) {
>> > }
>> >
>> > for (i = 0; i < n_target_int; i++) {
>> > + if (var_target_int[i] in var_target_explicit_mask) {
>> > + print " ptr->explicit_mask_" var_target_int[i] " =
>opts_set->x_" var_target_int[i] ";";
>> > + continue;
>> > + }
>> >print " if (opts_set->x_" var_target_int[i] ") mask |=
>HOST_WIDE_INT_1U << " j ";";
>> >j++;
>> >if (j == 64) {
>> > @@ -739,6 +752,10 @@ for (i = 0; i < n_extra_target_vars; i++
>> > }
>> >
>> > for (i = 0; i < n_target_other;
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Fri, Oct 02, 2020 at 04:21:12PM +0200, Stefan Schulze Frielinghaus via
Gcc-patches wrote:
> > > Sure, no problem at all. In that case I stop to investigate further and
> > > wait for you.
> >
> > Here is a patch that implements that.
> >
> > Can you please check if it fixes the s390x regressions that I couldn't
> > reproduce in a cross?
>
> Bootstrapped and regtested on S/390. Now all tattr-*.c test cases run
> successfully with the patch. All other tests remain the same.
I've now also successfully tested it on
{x86_64,i686,armv7hl,aarch64,powerpc64le,s390x. Ok for trunk?
> > 2020-10-02 Jakub Jelinek
> >
> > * opth-gen.awk: For variables referenced in Mask and InverseMask,
> > don't use the explicit_mask bitmask array, but add separate
> > explicit_mask_* members with the same types as the variables.
> > * optc-save-gen.awk: Save, restore, compare and hash the separate
> > explicit_mask_* members.
> >
> > --- gcc/opth-gen.awk.jj 2020-09-14 09:04:35.866854351 +0200
> > +++ gcc/opth-gen.awk2020-10-01 21:52:30.855122749 +0200
> > @@ -209,6 +209,7 @@ n_target_int = 0;
> > n_target_enum = 0;
> > n_target_other = 0;
> > n_target_explicit = n_extra_target_vars;
> > +n_target_explicit_mask = 0;
> >
> > for (i = 0; i < n_target_save; i++) {
> > if (target_save_decl[i] ~ "^((un)?signed +)?int +[_" alnum "]+$")
> > @@ -240,6 +241,12 @@ if (have_save) {
> > var_save_seen[name]++;
> > n_target_explicit++;
> > otype = var_type_struct(flags[i])
> > +
> > + if (opt_args("Mask", flags[i]) != "" \
> > + || opt_args("InverseMask", flags[i]))
> > +
> > var_target_explicit_mask[n_target_explicit_mask++] \
> > + = otype "explicit_mask_" name;
> > +
> > if (otype ~ "^((un)?signed +)?int *$")
> > var_target_int[n_target_int++] = otype "x_"
> > name;
> >
> > @@ -259,6 +266,8 @@ if (have_save) {
> > } else {
> > var_target_int[n_target_int++] = "int x_target_flags";
> > n_target_explicit++;
> > + var_target_explicit_mask[n_target_explicit_mask++] \
> > + = "int explicit_mask_target_flags";
> > }
> >
> > for (i = 0; i < n_target_other; i++) {
> > @@ -281,8 +290,12 @@ for (i = 0; i < n_target_char; i++) {
> > print " " var_target_char[i] ";";
> > }
> >
> > -print " /* " n_target_explicit " members */";
> > -print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit +
> > 63) / 64) "];";
> > +print " /* " n_target_explicit - n_target_explicit_mask " members */";
> > +print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit -
> > n_target_explicit_mask + 63) / 64) "];";
> > +
> > +for (i = 0; i < n_target_explicit_mask; i++) {
> > + print " " var_target_explicit_mask[i] ";";
> > +}
> >
> > print "};";
> > print "";
> > --- gcc/optc-save-gen.awk.jj2020-09-16 10:06:23.018093486 +0200
> > +++ gcc/optc-save-gen.awk 2020-10-01 21:48:10.933868862 +0200
> > @@ -516,6 +516,10 @@ if (have_save) {
> >
> > var_save_seen[name]++;
> > otype = var_type_struct(flags[i])
> > + if (opt_args("Mask", flags[i]) != "" \
> > + || opt_args("InverseMask", flags[i]))
> > + var_target_explicit_mask[name] = 1;
> > +
> > if (otype ~ "^((un)?signed +)?int *$")
> > var_target_int[n_target_int++] = name;
> >
> > @@ -545,6 +549,7 @@ if (have_save) {
> > }
> > } else {
> > var_target_int[n_target_int++] = "target_flags";
> > + var_target_explicit_mask["target_flags"] = 1;
> > }
> >
> > have_assert = 0;
> > @@ -608,6 +613,10 @@ for (i = 0; i < n_extra_target_vars; i++
> > }
> >
> > for (i = 0; i < n_target_other; i++) {
> > + if (var_target_other[i] in var_target_explicit_mask) {
> > + print " ptr->explicit_mask_" var_target_other[i] " =
> > opts_set->x_" var_target_other[i] ";";
> > + continue;
> > + }
> > print " if (opts_set->x_" var_target_other[i] ") mask |=
> > HOST_WIDE_INT_1U << " j ";";
> > j++;
> > if (j == 64) {
> > @@ -630,6 +639,10 @@ for (i = 0; i < n_target_enum; i++) {
> > }
> >
> > for (i = 0; i < n_target_int; i++) {
> > + if (var_target_int[i] in var_target_explicit_mask) {
> > + print " ptr->explicit_mask_" var_target_int[i] " =
> > opts_set->x_" var_target_int[i] ";";
> > + continue;
> > + }
> > print " if (opts_set->x_" var_target_int[i] ") mask |=
> > HOST_WIDE_INT_1U << " j ";";
> > j++;
> > if (j == 64) {
> > @@ -739,6 +752,10 @@ for (i = 0; i < n_extra_target_vars; i++
> > }
> >
> > for (i = 0; i < n_target_other; i++) {
> > + if (var_target_other[i] in var_target_explicit_mask) {
> > + print " opts_set->x_" var_target_other[i] " =
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Fri, Oct 02, 2020 at 10:46:33AM +0200, Jakub Jelinek wrote:
> On Wed, Sep 30, 2020 at 03:24:08PM +0200, Stefan Schulze Frielinghaus via
> Gcc-patches wrote:
> > On Wed, Sep 30, 2020 at 01:39:11PM +0200, Jakub Jelinek wrote:
> > > On Wed, Sep 30, 2020 at 01:21:44PM +0200, Stefan Schulze Frielinghaus
> > > wrote:
> > > > I think the problem boils down that on S/390 we distinguish between four
> > > > states of a flag: explicitely set to yes/no and implicitely set to
> > > > yes/no. If set explicitely, the option wins. For example, the options
> > > > `-march=z10 -mhtm` should enable the hardware transactional memory
> > > > option although z10 does not have one. In the past if a flag was set or
> > > > not explicitely was encoded into opts_set->x_target_flags ... for each
> > > > flag individually, e.g. TARGET_OPT_HTM_P (opts_set->x_target_flags) was
> > >
> > > Oops, seems I've missed that set_option has special treatment for
> > > CLVC_BIT_CLEAR/CLVC_BIT_SET.
> > > Which means I'll need to change the generic handling, so that for
> > > global_options_set elements mentioned in CLVC_BIT_* options are treated
> > > differently, instead of using the accumulated bitmasks they'll need to use
> > > their specific bitmask variables during the option saving/restoring.
> > > Is it ok if I defer it for tomorrow? Need to prepare for OpenMP meeting
> > > now.
> >
> > Sure, no problem at all. In that case I stop to investigate further and
> > wait for you.
>
> Here is a patch that implements that.
>
> Can you please check if it fixes the s390x regressions that I couldn't
> reproduce in a cross?
Bootstrapped and regtested on S/390. Now all tattr-*.c test cases run
successfully with the patch. All other tests remain the same.
Thanks for the quick follow up!
Cheers,
Stefan
>
> Bootstrapped/regtested on x86_64-linux and i686-linux so far.
> I don't have a convenient way to test it on the trunk on other
> architectures ATM, so I've just started testing a backport of the patchset to
> 10
> on {x86_64,i686,powerpc64le,s390x,armv7hl,aarch64}-linux (though, don't
> intend to actually commit the backport).
>
> 2020-10-02 Jakub Jelinek
>
> * opth-gen.awk: For variables referenced in Mask and InverseMask,
> don't use the explicit_mask bitmask array, but add separate
> explicit_mask_* members with the same types as the variables.
> * optc-save-gen.awk: Save, restore, compare and hash the separate
> explicit_mask_* members.
>
> --- gcc/opth-gen.awk.jj 2020-09-14 09:04:35.866854351 +0200
> +++ gcc/opth-gen.awk 2020-10-01 21:52:30.855122749 +0200
> @@ -209,6 +209,7 @@ n_target_int = 0;
> n_target_enum = 0;
> n_target_other = 0;
> n_target_explicit = n_extra_target_vars;
> +n_target_explicit_mask = 0;
>
> for (i = 0; i < n_target_save; i++) {
> if (target_save_decl[i] ~ "^((un)?signed +)?int +[_" alnum "]+$")
> @@ -240,6 +241,12 @@ if (have_save) {
> var_save_seen[name]++;
> n_target_explicit++;
> otype = var_type_struct(flags[i])
> +
> + if (opt_args("Mask", flags[i]) != "" \
> + || opt_args("InverseMask", flags[i]))
> +
> var_target_explicit_mask[n_target_explicit_mask++] \
> + = otype "explicit_mask_" name;
> +
> if (otype ~ "^((un)?signed +)?int *$")
> var_target_int[n_target_int++] = otype "x_"
> name;
>
> @@ -259,6 +266,8 @@ if (have_save) {
> } else {
> var_target_int[n_target_int++] = "int x_target_flags";
> n_target_explicit++;
> + var_target_explicit_mask[n_target_explicit_mask++] \
> + = "int explicit_mask_target_flags";
> }
>
> for (i = 0; i < n_target_other; i++) {
> @@ -281,8 +290,12 @@ for (i = 0; i < n_target_char; i++) {
> print " " var_target_char[i] ";";
> }
>
> -print " /* " n_target_explicit " members */";
> -print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit +
> 63) / 64) "];";
> +print " /* " n_target_explicit - n_target_explicit_mask " members */";
> +print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit -
> n_target_explicit_mask + 63) / 64) "];";
> +
> +for (i = 0; i < n_target_explicit_mask; i++) {
> + print " " var_target_explicit_mask[i] ";";
> +}
>
> print "};";
> print "";
> --- gcc/optc-save-gen.awk.jj 2020-09-16 10:06:23.018093486 +0200
> +++ gcc/optc-save-gen.awk 2020-10-01 21:48:10.933868862 +0200
> @@ -516,6 +516,10 @@ if (have_save) {
>
> var_save_seen[name]++;
> otype = var_type_struct(flags[i])
> + if (opt_args("Mask", flags[i]) != "" \
> + || opt_args("InverseMask", flags[i]))
> + var_target_explicit_mask[name] = 1;
> +
> if (otype ~ "^((un)?signed +)?int *$")
>
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Wed, Sep 30, 2020 at 03:24:08PM +0200, Stefan Schulze Frielinghaus via
Gcc-patches wrote:
> On Wed, Sep 30, 2020 at 01:39:11PM +0200, Jakub Jelinek wrote:
> > On Wed, Sep 30, 2020 at 01:21:44PM +0200, Stefan Schulze Frielinghaus wrote:
> > > I think the problem boils down that on S/390 we distinguish between four
> > > states of a flag: explicitely set to yes/no and implicitely set to
> > > yes/no. If set explicitely, the option wins. For example, the options
> > > `-march=z10 -mhtm` should enable the hardware transactional memory
> > > option although z10 does not have one. In the past if a flag was set or
> > > not explicitely was encoded into opts_set->x_target_flags ... for each
> > > flag individually, e.g. TARGET_OPT_HTM_P (opts_set->x_target_flags) was
> >
> > Oops, seems I've missed that set_option has special treatment for
> > CLVC_BIT_CLEAR/CLVC_BIT_SET.
> > Which means I'll need to change the generic handling, so that for
> > global_options_set elements mentioned in CLVC_BIT_* options are treated
> > differently, instead of using the accumulated bitmasks they'll need to use
> > their specific bitmask variables during the option saving/restoring.
> > Is it ok if I defer it for tomorrow? Need to prepare for OpenMP meeting now.
>
> Sure, no problem at all. In that case I stop to investigate further and
> wait for you.
Here is a patch that implements that.
Can you please check if it fixes the s390x regressions that I couldn't
reproduce in a cross?
Bootstrapped/regtested on x86_64-linux and i686-linux so far.
I don't have a convenient way to test it on the trunk on other
architectures ATM, so I've just started testing a backport of the patchset to 10
on {x86_64,i686,powerpc64le,s390x,armv7hl,aarch64}-linux (though, don't
intend to actually commit the backport).
2020-10-02 Jakub Jelinek
* opth-gen.awk: For variables referenced in Mask and InverseMask,
don't use the explicit_mask bitmask array, but add separate
explicit_mask_* members with the same types as the variables.
* optc-save-gen.awk: Save, restore, compare and hash the separate
explicit_mask_* members.
--- gcc/opth-gen.awk.jj 2020-09-14 09:04:35.866854351 +0200
+++ gcc/opth-gen.awk2020-10-01 21:52:30.855122749 +0200
@@ -209,6 +209,7 @@ n_target_int = 0;
n_target_enum = 0;
n_target_other = 0;
n_target_explicit = n_extra_target_vars;
+n_target_explicit_mask = 0;
for (i = 0; i < n_target_save; i++) {
if (target_save_decl[i] ~ "^((un)?signed +)?int +[_" alnum "]+$")
@@ -240,6 +241,12 @@ if (have_save) {
var_save_seen[name]++;
n_target_explicit++;
otype = var_type_struct(flags[i])
+
+ if (opt_args("Mask", flags[i]) != "" \
+ || opt_args("InverseMask", flags[i]))
+
var_target_explicit_mask[n_target_explicit_mask++] \
+ = otype "explicit_mask_" name;
+
if (otype ~ "^((un)?signed +)?int *$")
var_target_int[n_target_int++] = otype "x_"
name;
@@ -259,6 +266,8 @@ if (have_save) {
} else {
var_target_int[n_target_int++] = "int x_target_flags";
n_target_explicit++;
+ var_target_explicit_mask[n_target_explicit_mask++] \
+ = "int explicit_mask_target_flags";
}
for (i = 0; i < n_target_other; i++) {
@@ -281,8 +290,12 @@ for (i = 0; i < n_target_char; i++) {
print " " var_target_char[i] ";";
}
-print " /* " n_target_explicit " members */";
-print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit + 63)
/ 64) "];";
+print " /* " n_target_explicit - n_target_explicit_mask " members */";
+print " unsigned HOST_WIDE_INT explicit_mask[" int ((n_target_explicit -
n_target_explicit_mask + 63) / 64) "];";
+
+for (i = 0; i < n_target_explicit_mask; i++) {
+ print " " var_target_explicit_mask[i] ";";
+}
print "};";
print "";
--- gcc/optc-save-gen.awk.jj2020-09-16 10:06:23.018093486 +0200
+++ gcc/optc-save-gen.awk 2020-10-01 21:48:10.933868862 +0200
@@ -516,6 +516,10 @@ if (have_save) {
var_save_seen[name]++;
otype = var_type_struct(flags[i])
+ if (opt_args("Mask", flags[i]) != "" \
+ || opt_args("InverseMask", flags[i]))
+ var_target_explicit_mask[name] = 1;
+
if (otype ~ "^((un)?signed +)?int *$")
var_target_int[n_target_int++] = name;
@@ -545,6 +549,7 @@ if (have_save) {
}
} else {
var_target_int[n_target_int++] = "target_flags";
+ var_target_explicit_mask["target_flags"] = 1;
}
have_assert = 0;
@@ -608,6 +613,10 @@ for (i = 0; i < n_extra_target_vars; i++
}
for (i = 0; i < n_target_other; i++) {
+ if (var_target_other[i] in
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Wed, Sep 30, 2020 at 01:39:11PM +0200, Jakub Jelinek wrote: > On Wed, Sep 30, 2020 at 01:21:44PM +0200, Stefan Schulze Frielinghaus wrote: > > I think the problem boils down that on S/390 we distinguish between four > > states of a flag: explicitely set to yes/no and implicitely set to > > yes/no. If set explicitely, the option wins. For example, the options > > `-march=z10 -mhtm` should enable the hardware transactional memory > > option although z10 does not have one. In the past if a flag was set or > > not explicitely was encoded into opts_set->x_target_flags ... for each > > flag individually, e.g. TARGET_OPT_HTM_P (opts_set->x_target_flags) was > > Oops, seems I've missed that set_option has special treatment for > CLVC_BIT_CLEAR/CLVC_BIT_SET. > Which means I'll need to change the generic handling, so that for > global_options_set elements mentioned in CLVC_BIT_* options are treated > differently, instead of using the accumulated bitmasks they'll need to use > their specific bitmask variables during the option saving/restoring. > Is it ok if I defer it for tomorrow? Need to prepare for OpenMP meeting now. Sure, no problem at all. In that case I stop to investigate further and wait for you. Cheers, Stefan
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Wed, Sep 30, 2020 at 01:21:44PM +0200, Stefan Schulze Frielinghaus wrote: > I think the problem boils down that on S/390 we distinguish between four > states of a flag: explicitely set to yes/no and implicitely set to > yes/no. If set explicitely, the option wins. For example, the options > `-march=z10 -mhtm` should enable the hardware transactional memory > option although z10 does not have one. In the past if a flag was set or > not explicitely was encoded into opts_set->x_target_flags ... for each > flag individually, e.g. TARGET_OPT_HTM_P (opts_set->x_target_flags) was Oops, seems I've missed that set_option has special treatment for CLVC_BIT_CLEAR/CLVC_BIT_SET. Which means I'll need to change the generic handling, so that for global_options_set elements mentioned in CLVC_BIT_* options are treated differently, instead of using the accumulated bitmasks they'll need to use their specific bitmask variables during the option saving/restoring. Is it ok if I defer it for tomorrow? Need to prepare for OpenMP meeting now. Jakub
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Wed, Sep 30, 2020 at 11:32:55AM +0200, Jakub Jelinek wrote:
> On Mon, Sep 28, 2020 at 09:50:00PM +0200, Stefan Schulze Frielinghaus via
> Gcc-patches wrote:
> > This patch breaks quite a view test cases (target-attribute/tattr-*) on
> > IBM Z. Having a look at function cl_target_option_restore reveals that
> > some members of opts_set are reduced to 1 or 0 depending on whether a
> > member was set before or not, e.g. for target_flags we have
>
> I've tried to reproduce the tattr FAILs reported in
> https://gcc.gnu.org/pipermail/gcc-testresults/2020-September/608760.html
> in a cross-compiler (with
> #define HAVE_AS_MACHINE_MACHINEMODE 1
> ), but couldn't, neither the ICEs nor the scan-assembler failures.
> Anyway, could you do a side-by-side debugging of one of those failures
> before/after my change and see what behaves differently?
I think the problem boils down that on S/390 we distinguish between four
states of a flag: explicitely set to yes/no and implicitely set to
yes/no. If set explicitely, the option wins. For example, the options
`-march=z10 -mhtm` should enable the hardware transactional memory
option although z10 does not have one. In the past if a flag was set or
not explicitely was encoded into opts_set->x_target_flags ... for each
flag individually, e.g. TARGET_OPT_HTM_P (opts_set->x_target_flags) was
used. This has changed with the mentioned patch in the sense that
opts_set encodes whether any flag of x_target_flags was set or not but
not which individual one after a call to the generated function
cl_target_option_restore where we have:
opts_set->x_target_flags = (mask & 1) != 0;
Compiling the following program
#pragma GCC target ("arch=z10")
void fn_pragma_0 (void) { }
with options `-march=z13 -mzarch -mhtm -mdebug` produces different flags
for 4ac7b669580 (commit prior your patch) and ba948b37768 (your patch).
This is my current understanding of the option handling. I will try to
come up with a trace where these things become hopefully more clear.
Cheers,
Stefan
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Mon, Sep 28, 2020 at 09:50:00PM +0200, Stefan Schulze Frielinghaus via Gcc-patches wrote: > This patch breaks quite a view test cases (target-attribute/tattr-*) on > IBM Z. Having a look at function cl_target_option_restore reveals that > some members of opts_set are reduced to 1 or 0 depending on whether a > member was set before or not, e.g. for target_flags we have I've tried to reproduce the tattr FAILs reported in https://gcc.gnu.org/pipermail/gcc-testresults/2020-September/608760.html in a cross-compiler (with #define HAVE_AS_MACHINE_MACHINEMODE 1 ), but couldn't, neither the ICEs nor the scan-assembler failures. Anyway, could you do a side-by-side debugging of one of those failures before/after my change and see what behaves differently? Jakub
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Mon, Sep 28, 2020 at 09:50:00PM +0200, Stefan Schulze Frielinghaus wrote: > This patch breaks quite a view test cases (target-attribute/tattr-*) on > IBM Z. Having a look at function cl_target_option_restore reveals that > some members of opts_set are reduced to 1 or 0 depending on whether a > member was set before or not, e.g. for target_flags we have > > opts_set->x_target_flags = (mask & 1) != 0; > > whereas previously those members where not touched by > cl_target_option_restore. opts_set is always used as a booleans, cleaner would be to have a struct global_option_set that would contain just bool fields (or even bitfields) with the same names as in struct global_option, but unfortunately we didn't do that years ago and changing it now would be quite a lot of work. Anyway, opts_set records a boolean for each flag, whether it was set explicitly or not, for non-pointer vars that means values 0 and 1, for pointer vars NULL and "". I vaguely remember the s390 opts_set handling differed quite a lot from all the other targets. > What puzzles me right now is that in cl_target_option_save we save in > ptr only options from opts but not from opts_set whereas in > cl_target_option_restore we override some members of opts_set. Thus it > is unclear to me how a backend should restore opts_set then. I've handled in the generic code handling of the opts_set only for whatever is handled there for opts too. Whether the vars that are handled in the save/restore options hooks need handling of opts_set at all or not depends on whether the backend ever cares about those or not (e.g. whether it every queries global_options_set.x_whatever). If yes, it can be handled e.g. by having an extra target variable that will store whether it was explicit or not and restore from there. If it is never used, then it doesn't need to be handled. Jakub
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Sun, Sep 13, 2020 at 10:29:22AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Sep 11, 2020 at 11:29:52AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > On Fri, Sep 11, 2020 at 09:46:37AM +0200, Christophe Lyon via Gcc-patches
> > wrote:
> > > I'm seeing an ICE with this new test on most of my arm configurations,
> > > for instance:
> > > --target arm-none-linux-gnueabi --with-cpu cortex-a9
> > > /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-linux-gnueabi/gcc3/gcc/xgcc
> > > -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-ar
> > > m-none-linux-gnueabi/gcc3/gcc/ c_lto_pr96939_0.o c_lto_pr96939_1.o
> > > -fdiagnostics-plain-output -flto -O2 -o
> > > gcc-target-arm-lto-pr96939-01.exe
> >
> > Seems a latent issue.
> > Neither cl_optimization_{save,restore} nor cl_target_option_{save,restore}
> > (nor any of the target hooks they call) saves or restores any opts_set
> > values, so I think opts_set can be trusted only during option processing (if
> > at all), but not later.
> > So, short term a fix would be IMHO just stop using opts_set altogether in
> > arm_configure_build_target, it doesn't make much sense to me, it should test
> > if those strings are non-NULL instead, or at least do that when it is
> > invoked from arm_option_restore (e.g. could be done by calling it with
> > opts instead of _options_set ).
> > Longer term, the question is if cl_optimization_{save,restore} and
> > cl_target_option_{save,restore} shouldn't be changed not to only
> > save/restore the options, but also save the opts_set flags.
> > It could be done e.g. by adding a bool array or set of bool members
> > to struct cl_optimization and struct cl_target_option , or even more compact
> > by using bitmasks, pack each 64 adjacent option flags into a UHWI element
> > of an array.
>
> So, I've tried under debugger how it behaves and seems global_options_set
> is really an or of whether an option has been ever seen as explicit, either
> on the command line or in any of the option pragmas or optimize/target
> attributes seen so far, so it isn't something that can be relied on.
>
> The following patch implements the saving/restoring of the opts_set bits
> (though only for the options/variables saved by the generic options-save.c
> code, for the target specific stuff that isn't handled by the generic code
> the opts_set argument is now passed to the hook and the backends can choose
> e.g. to use a TargetSave variable to save the flags either individually or
> together in some bitmask (or ignore it if they never need opts_set for the
> options).
>
> Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
> aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
> for trunk?
This patch breaks quite a view test cases (target-attribute/tattr-*) on
IBM Z. Having a look at function cl_target_option_restore reveals that
some members of opts_set are reduced to 1 or 0 depending on whether a
member was set before or not, e.g. for target_flags we have
opts_set->x_target_flags = (mask & 1) != 0;
whereas previously those members where not touched by
cl_target_option_restore.
My intuition of this whole option evaluation is still pretty vague.
Basically opts_set is a set of options enabled via command line and/or
via pragmas/attributes whereas opts is the set of options which are
implied by opts_set.
What puzzles me right now is that in cl_target_option_save we save in
ptr only options from opts but not from opts_set whereas in
cl_target_option_restore we override some members of opts_set. Thus it
is unclear to me how a backend should restore opts_set then.
I'm probably missing something. Any hints on how to restore opts_set
and especially target_flags?
Cheers,
Stefan
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Mon, 14 Sep 2020 at 08:33, Richard Biener wrote:
>
> On Sun, 13 Sep 2020, Jakub Jelinek wrote:
>
> > On Fri, Sep 11, 2020 at 11:29:52AM +0200, Jakub Jelinek via Gcc-patches
> > wrote:
> > > On Fri, Sep 11, 2020 at 09:46:37AM +0200, Christophe Lyon via Gcc-patches
> > > wrote:
> > > > I'm seeing an ICE with this new test on most of my arm configurations,
> > > > for instance:
> > > > --target arm-none-linux-gnueabi --with-cpu cortex-a9
> > > > /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-linux-gnueabi/gcc3/gcc/xgcc
> > > > -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-ar
> > > > m-none-linux-gnueabi/gcc3/gcc/ c_lto_pr96939_0.o c_lto_pr96939_1.o
> > > > -fdiagnostics-plain-output -flto -O2 -o
> > > > gcc-target-arm-lto-pr96939-01.exe
> > >
> > > Seems a latent issue.
> > > Neither cl_optimization_{save,restore} nor cl_target_option_{save,restore}
> > > (nor any of the target hooks they call) saves or restores any opts_set
> > > values, so I think opts_set can be trusted only during option processing
> > > (if
> > > at all), but not later.
> > > So, short term a fix would be IMHO just stop using opts_set altogether in
> > > arm_configure_build_target, it doesn't make much sense to me, it should
> > > test
> > > if those strings are non-NULL instead, or at least do that when it is
> > > invoked from arm_option_restore (e.g. could be done by calling it with
> > > opts instead of _options_set ).
> > > Longer term, the question is if cl_optimization_{save,restore} and
> > > cl_target_option_{save,restore} shouldn't be changed not to only
> > > save/restore the options, but also save the opts_set flags.
> > > It could be done e.g. by adding a bool array or set of bool members
> > > to struct cl_optimization and struct cl_target_option , or even more
> > > compact
> > > by using bitmasks, pack each 64 adjacent option flags into a UHWI element
> > > of an array.
> >
> > So, I've tried under debugger how it behaves and seems global_options_set
> > is really an or of whether an option has been ever seen as explicit, either
> > on the command line or in any of the option pragmas or optimize/target
> > attributes seen so far, so it isn't something that can be relied on.
> >
> > The following patch implements the saving/restoring of the opts_set bits
> > (though only for the options/variables saved by the generic options-save.c
> > code, for the target specific stuff that isn't handled by the generic code
> > the opts_set argument is now passed to the hook and the backends can choose
> > e.g. to use a TargetSave variable to save the flags either individually or
> > together in some bitmask (or ignore it if they never need opts_set for the
> > options).
> >
> > Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
> > aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
> > for trunk?
>
Hi Jakub,
This patch is causing build failures in libgcc when targeting
arm-none-eabi with multilibs enabled:
I see an error with -marm -mfpu=auto -march=armv5te+fp -mfloat-abi=hard
when compiling /libgcc/config/arm/pr-support.c:
/tmp/5619380_8.tmpdir/ccxsqYqf.s: Assembler messages:
/tmp/5619380_8.tmpdir/ccxsqYqf.s:290: Error: selected processor does
not support `uxtb r3,r3' in ARM mode
/tmp/5619380_8.tmpdir/ccxsqYqf.s:387: Error: selected processor does
not support `uxth r2,r4' in ARM mode
and similar with /libgcc/config/arm/unwind-arm.c:
/tmp/5619380_8.tmpdir/ccB9Efuf.s: Assembler messages:
/tmp/5619380_8.tmpdir/ccB9Efuf.s:396: Error: selected processor does
not support `ubfx r0,r3,#24,#4' in ARM mode
/tmp/5619380_8.tmpdir/ccB9Efuf.s:852: Error: selected processor does
not support `uxtb r10,r9' in ARM mode
/tmp/5619380_8.tmpdir/ccB9Efuf.s:1776: Error: selected processor does
not support `uxtb r2,r2' in ARM mode
/tmp/5619380_8.tmpdir/ccB9Efuf.s:2658: Error: selected processor does
not support `uxth r4,r2' in ARM mode
/tmp/5619380_8.tmpdir/ccB9Efuf.s:2751: Error: selected processor does
not support `uxth r5,r2' in ARM mode
/tmp/5619380_8.tmpdir/ccB9Efuf.s:2914: Error: selected processor does
not support `uxth ip,r2' in ARM mode
Looks like you opened a can of worms :-)
> OK.
>
> Richard.
>
> > This patch itself doesn't fix the testcase failing on arm, but a follow up
> > patch will.
> >
> > 2020-09-13 Jakub Jelinek
> >
> > gcc/
> > * opt-read.awk: Also initialize extra_target_var_types array.
> > * opth-gen.awk: Emit explicit_mask arrays to struct cl_optimization
> > and cl_target_option. Adjust cl_optimization_save,
> > cl_optimization_restore, cl_target_option_save and
> > cl_target_option_restore declarations.
> > * optc-save-gen.awk: Add opts_set argument to cl_optimization_save,
> > cl_optimization_restore, cl_target_option_save and
> > cl_target_option_restore functions and save or restore opts_set
> > next to the opts values into or from explicit_mask arrays.
> > In cl_target_option_eq and cl_optimization_option_eq
Re: [PATCH] options: Save and restore opts_set for Optimization and Target options
On Sun, 13 Sep 2020, Jakub Jelinek wrote:
> On Fri, Sep 11, 2020 at 11:29:52AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > On Fri, Sep 11, 2020 at 09:46:37AM +0200, Christophe Lyon via Gcc-patches
> > wrote:
> > > I'm seeing an ICE with this new test on most of my arm configurations,
> > > for instance:
> > > --target arm-none-linux-gnueabi --with-cpu cortex-a9
> > > /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-linux-gnueabi/gcc3/gcc/xgcc
> > > -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-ar
> > > m-none-linux-gnueabi/gcc3/gcc/ c_lto_pr96939_0.o c_lto_pr96939_1.o
> > > -fdiagnostics-plain-output -flto -O2 -o
> > > gcc-target-arm-lto-pr96939-01.exe
> >
> > Seems a latent issue.
> > Neither cl_optimization_{save,restore} nor cl_target_option_{save,restore}
> > (nor any of the target hooks they call) saves or restores any opts_set
> > values, so I think opts_set can be trusted only during option processing (if
> > at all), but not later.
> > So, short term a fix would be IMHO just stop using opts_set altogether in
> > arm_configure_build_target, it doesn't make much sense to me, it should test
> > if those strings are non-NULL instead, or at least do that when it is
> > invoked from arm_option_restore (e.g. could be done by calling it with
> > opts instead of _options_set ).
> > Longer term, the question is if cl_optimization_{save,restore} and
> > cl_target_option_{save,restore} shouldn't be changed not to only
> > save/restore the options, but also save the opts_set flags.
> > It could be done e.g. by adding a bool array or set of bool members
> > to struct cl_optimization and struct cl_target_option , or even more compact
> > by using bitmasks, pack each 64 adjacent option flags into a UHWI element
> > of an array.
>
> So, I've tried under debugger how it behaves and seems global_options_set
> is really an or of whether an option has been ever seen as explicit, either
> on the command line or in any of the option pragmas or optimize/target
> attributes seen so far, so it isn't something that can be relied on.
>
> The following patch implements the saving/restoring of the opts_set bits
> (though only for the options/variables saved by the generic options-save.c
> code, for the target specific stuff that isn't handled by the generic code
> the opts_set argument is now passed to the hook and the backends can choose
> e.g. to use a TargetSave variable to save the flags either individually or
> together in some bitmask (or ignore it if they never need opts_set for the
> options).
>
> Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
> aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
> for trunk?
OK.
Richard.
> This patch itself doesn't fix the testcase failing on arm, but a follow up
> patch will.
>
> 2020-09-13 Jakub Jelinek
>
> gcc/
> * opt-read.awk: Also initialize extra_target_var_types array.
> * opth-gen.awk: Emit explicit_mask arrays to struct cl_optimization
> and cl_target_option. Adjust cl_optimization_save,
> cl_optimization_restore, cl_target_option_save and
> cl_target_option_restore declarations.
> * optc-save-gen.awk: Add opts_set argument to cl_optimization_save,
> cl_optimization_restore, cl_target_option_save and
> cl_target_option_restore functions and save or restore opts_set
> next to the opts values into or from explicit_mask arrays.
> In cl_target_option_eq and cl_optimization_option_eq compare
> explicit_mask arrays, in cl_target_option_hash and cl_optimization_hash
> hash them and in cl_target_option_stream_out,
> cl_target_option_stream_in, cl_optimization_stream_out and
> cl_optimization_stream_in stream them.
> * tree.h (build_optimization_node, build_target_option_node): Add
> opts_set argument.
> * tree.c (build_optimization_node): Add opts_set argument, pass it
> to cl_optimization_save.
> (build_target_option_node): Add opts_set argument, pass it to
> cl_target_option_save.
> * function.c (invoke_set_current_function_hook): Adjust
> cl_optimization_restore caller.
> * ipa-inline-transform.c (inline_call): Adjust cl_optimization_restore
> and build_optimization_node callers.
> * target.def (TARGET_OPTION_SAVE, TARGET_OPTION_RESTORE): Add opts_set
> argument.
> * target-globals.c (save_target_globals_default_opts): Adjust
> cl_optimization_restore callers.
> * toplev.c (process_options): Adjust build_optimization_node and
> cl_optimization_restore callers.
> (target_reinit): Adjust cl_optimization_restore caller.
> * tree-streamer-in.c (lto_input_ts_function_decl_tree_pointers):
> Adjust build_optimization_node and cl_optimization_restore callers.
> * doc/tm.texi: Updated.
> * config/aarch64/aarch64.c (aarch64_override_options): Adjust
> build_target_option_node caller.
> (aarch64_option_save,
[PATCH] options: Save and restore opts_set for Optimization and Target options
On Fri, Sep 11, 2020 at 11:29:52AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Sep 11, 2020 at 09:46:37AM +0200, Christophe Lyon via Gcc-patches
> wrote:
> > I'm seeing an ICE with this new test on most of my arm configurations,
> > for instance:
> > --target arm-none-linux-gnueabi --with-cpu cortex-a9
> > /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-linux-gnueabi/gcc3/gcc/xgcc
> > -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-ar
> > m-none-linux-gnueabi/gcc3/gcc/ c_lto_pr96939_0.o c_lto_pr96939_1.o
> > -fdiagnostics-plain-output -flto -O2 -o
> > gcc-target-arm-lto-pr96939-01.exe
>
> Seems a latent issue.
> Neither cl_optimization_{save,restore} nor cl_target_option_{save,restore}
> (nor any of the target hooks they call) saves or restores any opts_set
> values, so I think opts_set can be trusted only during option processing (if
> at all), but not later.
> So, short term a fix would be IMHO just stop using opts_set altogether in
> arm_configure_build_target, it doesn't make much sense to me, it should test
> if those strings are non-NULL instead, or at least do that when it is
> invoked from arm_option_restore (e.g. could be done by calling it with
> opts instead of _options_set ).
> Longer term, the question is if cl_optimization_{save,restore} and
> cl_target_option_{save,restore} shouldn't be changed not to only
> save/restore the options, but also save the opts_set flags.
> It could be done e.g. by adding a bool array or set of bool members
> to struct cl_optimization and struct cl_target_option , or even more compact
> by using bitmasks, pack each 64 adjacent option flags into a UHWI element
> of an array.
So, I've tried under debugger how it behaves and seems global_options_set
is really an or of whether an option has been ever seen as explicit, either
on the command line or in any of the option pragmas or optimize/target
attributes seen so far, so it isn't something that can be relied on.
The following patch implements the saving/restoring of the opts_set bits
(though only for the options/variables saved by the generic options-save.c
code, for the target specific stuff that isn't handled by the generic code
the opts_set argument is now passed to the hook and the backends can choose
e.g. to use a TargetSave variable to save the flags either individually or
together in some bitmask (or ignore it if they never need opts_set for the
options).
Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
for trunk?
This patch itself doesn't fix the testcase failing on arm, but a follow up
patch will.
2020-09-13 Jakub Jelinek
gcc/
* opt-read.awk: Also initialize extra_target_var_types array.
* opth-gen.awk: Emit explicit_mask arrays to struct cl_optimization
and cl_target_option. Adjust cl_optimization_save,
cl_optimization_restore, cl_target_option_save and
cl_target_option_restore declarations.
* optc-save-gen.awk: Add opts_set argument to cl_optimization_save,
cl_optimization_restore, cl_target_option_save and
cl_target_option_restore functions and save or restore opts_set
next to the opts values into or from explicit_mask arrays.
In cl_target_option_eq and cl_optimization_option_eq compare
explicit_mask arrays, in cl_target_option_hash and cl_optimization_hash
hash them and in cl_target_option_stream_out,
cl_target_option_stream_in, cl_optimization_stream_out and
cl_optimization_stream_in stream them.
* tree.h (build_optimization_node, build_target_option_node): Add
opts_set argument.
* tree.c (build_optimization_node): Add opts_set argument, pass it
to cl_optimization_save.
(build_target_option_node): Add opts_set argument, pass it to
cl_target_option_save.
* function.c (invoke_set_current_function_hook): Adjust
cl_optimization_restore caller.
* ipa-inline-transform.c (inline_call): Adjust cl_optimization_restore
and build_optimization_node callers.
* target.def (TARGET_OPTION_SAVE, TARGET_OPTION_RESTORE): Add opts_set
argument.
* target-globals.c (save_target_globals_default_opts): Adjust
cl_optimization_restore callers.
* toplev.c (process_options): Adjust build_optimization_node and
cl_optimization_restore callers.
(target_reinit): Adjust cl_optimization_restore caller.
* tree-streamer-in.c (lto_input_ts_function_decl_tree_pointers):
Adjust build_optimization_node and cl_optimization_restore callers.
* doc/tm.texi: Updated.
* config/aarch64/aarch64.c (aarch64_override_options): Adjust
build_target_option_node caller.
(aarch64_option_save, aarch64_option_restore): Add opts_set argument.
(aarch64_set_current_function): Adjust cl_target_option_restore
caller.
