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  <ja...@redhat.com>
> 
>       * 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 *$")
>                               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] " = 
> ptr->explicit_mask_" var_target_other[i] ";";
> +             continue;
> +     }
>       if (j == 64) {
>               print "  mask = ptr->explicit_mask[" k "];";
>               k++;
> @@ -761,6 +778,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 "  opts_set->x_" var_target_int[i] " = 
> ptr->explicit_mask_" var_target_int[i] ";";
> +             continue;
> +     }
>       if (j == 64) {
>               print "  mask = ptr->explicit_mask[" k "];";
>               k++;
> @@ -1058,6 +1079,20 @@ print "  for (size_t i = 0; i < sizeof (
>  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) {
> +             print "  if (ptr1->explicit_mask_" var_target_other[i] " != 
> ptr2->explicit_mask_" var_target_other[i] ")";
> +             print "    return false;";
> +     }
> +}
> +
> +for (i = 0; i < n_target_int; i++) {
> +     if (var_target_int[i] in var_target_explicit_mask) {
> +             print "  if (ptr1->explicit_mask_" var_target_int[i] " != 
> ptr2->explicit_mask_" var_target_int[i] ")";
> +             print "    return false;";
> +     }
> +}
> +
>  print "  return true;";
>  
>  print "}";
> @@ -1088,6 +1123,17 @@ for (i = 0; i < n_target_val; i++) {
>  }
>  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)
> +             print "  hstate.add_hwi (ptr->explicit_mask_" 
> var_target_other[i] ");";
> +}
> +
> +for (i = 0; i < n_target_int; i++) {
> +     if (var_target_int[i] in var_target_explicit_mask)
> +             print "  hstate.add_hwi (ptr->explicit_mask_" var_target_int[i] 
> ");";
> +}
> +
>  print "  return hstate.end ();";
>  print "}";
>  
> 
> 
>       Jakub
> 

Reply via email to