Re: Backport [PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-06-28 Thread Segher Boessenkool 
On Wed, Jun 28, 2017 at 08:30:21AM -0600, Kelvin Nilsen wrote:
> I have bootstrapped and tested this patch on
> powerpc64le-unkonwn-linux-gnu with no regressions.  Is this ok for
> backporting to gcc 6?

Please wait until after 6.4.

Thanks,


Segher

Re: Backport [PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-06-28 Thread Kelvin Nilsen 

I have bootstrapped and tested this patch on
powerpc64le-unkonwn-linux-gnu with no regressions.  Is this ok for
backporting to gcc 6?



On 03/22/2017 10:17 PM, Segher Boessenkool wrote:
> On Wed, Mar 22, 2017 at 05:55:53PM -0600, Kelvin Nilsen wrote:
>>> Or it could do -mpower9-dform-scalar but disable -mpower9-dform-vector?
>>> That seems more reasonable.
>>
>> The internal problem report sent to me said "-mno-power9-vector should
>> override power9-dform unless the latter has been deliberately specified
>> by the user."  I'm just following orders.
> 
> Heh :-)
> 
>> If you think it preferable to
>> only override -mpower-dform-vector, I'll make that modification.
> 
> It is more logical.  Or so I though.  But as it turns out,
> -mpower9-dform-scalar is about vector registers as well.
> 
> So the patch is approved for trunk as-is.  Thanks!
> 
* config/rs6000/rs6000.c (rs6000_option_override_internal): Change
handling of certain combinations of target options, including the
combinations -mpower8-vector vs. -mno-vsx, -mpower8-vector vs.
-mno-power8-vector, and -mpower9_dform vs. -mno-power9-vector.
>>>
>>> Those other changes are independent?
>>
>> Actually, these other changes are not independent.  My initial attempt
>> at a patch only changed the behavior of -mpower9_dform vs.
>> -mno-power9-vector.  But this actually resulted in a regression of an
>> existing test.  To "properly" handle the new case without impacting
>> existing "established" behavior (as represented in the existing dejagnu
>> testsuite), I had to make these other changes as well.
> 
> Too many options :-(
> 
> 
> Segher
> 
> 

-- 
Kelvin Nilsen, Ph.D.  kdnil...@linux.vnet.ibm.com
home office: 801-756-4821, cell: 520-991-6727
IBM Linux Technology Center - PPC Toolchain

Re: [PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-03-22 Thread Segher Boessenkool 
On Wed, Mar 22, 2017 at 05:55:53PM -0600, Kelvin Nilsen wrote:
> > Or it could do -mpower9-dform-scalar but disable -mpower9-dform-vector?
> > That seems more reasonable.
> 
> The internal problem report sent to me said "-mno-power9-vector should
> override power9-dform unless the latter has been deliberately specified
> by the user."  I'm just following orders.

Heh :-)

> If you think it preferable to
> only override -mpower-dform-vector, I'll make that modification.

It is more logical.  Or so I though.  But as it turns out,
-mpower9-dform-scalar is about vector registers as well.

So the patch is approved for trunk as-is.  Thanks!

> >>* config/rs6000/rs6000.c (rs6000_option_override_internal): Change
> >>handling of certain combinations of target options, including the
> >>combinations -mpower8-vector vs. -mno-vsx, -mpower8-vector vs.
> >>-mno-power8-vector, and -mpower9_dform vs. -mno-power9-vector.
> > 
> > Those other changes are independent?
> 
> Actually, these other changes are not independent.  My initial attempt
> at a patch only changed the behavior of -mpower9_dform vs.
> -mno-power9-vector.  But this actually resulted in a regression of an
> existing test.  To "properly" handle the new case without impacting
> existing "established" behavior (as represented in the existing dejagnu
> testsuite), I had to make these other changes as well.

Too many options :-(


Segher

Re: [PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-03-22 Thread Kelvin Nilsen 


On 03/22/2017 05:35 PM, Segher Boessenkool wrote:
> On Wed, Mar 22, 2017 at 11:44:49AM -0600, Kelvin Nilsen wrote:
>> Internal testing recently revealed that use of the -mno-power9-vector
>> target option in combination with the -mcpu=power9 target option
>> results in termination of gcc with the error message:
>>
>>   power9-dform requires power9-vector
> 
>> In both cases, the preferred behavior is that the target option
>> -mno-power9-vector causes power9-dform to be automatically disabled.
>>  This patch implements the preferred behavior and adds a test case to
>> demonstrate the fix.
> 
> Or it could do -mpower9-dform-scalar but disable -mpower9-dform-vector?
> That seems more reasonable.

The internal problem report sent to me said "-mno-power9-vector should
override power9-dform unless the latter has been deliberately specified
by the user."  I'm just following orders.  If you think it preferable to
only override -mpower-dform-vector, I'll make that modification.

> 
> Ideally none of the -mpower9-dform* or -mpower9-vector options would
> exist at all, of course.
> 
>> 2017-03-21  Kelvin Nilsen  
>>
>>  * config/rs6000/rs6000.c (rs6000_option_override_internal): Change
>>  handling of certain combinations of target options, including the
>>  combinations -mpower8-vector vs. -mno-vsx, -mpower8-vector vs.
>>  -mno-power8-vector, and -mpower9_dform vs. -mno-power9-vector.
> 
> Those other changes are independent?

Actually, these other changes are not independent.  My initial attempt
at a patch only changed the behavior of -mpower9_dform vs.
-mno-power9-vector.  But this actually resulted in a regression of an
existing test.  To "properly" handle the new case without impacting
existing "established" behavior (as represented in the existing dejagnu
testsuite), I had to make these other changes as well.


> 
> 
> Segher
> 
> 

-- 
Kelvin Nilsen, Ph.D.  kdnil...@linux.vnet.ibm.com
home office: 801-756-4821, cell: 520-991-6727
IBM Linux Technology Center - PPC Toolchain

Re: [PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-03-22 Thread Segher Boessenkool 
On Wed, Mar 22, 2017 at 11:44:49AM -0600, Kelvin Nilsen wrote:
> Internal testing recently revealed that use of the -mno-power9-vector
> target option in combination with the -mcpu=power9 target option
> results in termination of gcc with the error message:
> 
>   power9-dform requires power9-vector

> In both cases, the preferred behavior is that the target option
> -mno-power9-vector causes power9-dform to be automatically disabled.
>  This patch implements the preferred behavior and adds a test case to
> demonstrate the fix.

Or it could do -mpower9-dform-scalar but disable -mpower9-dform-vector?
That seems more reasonable.

Ideally none of the -mpower9-dform* or -mpower9-vector options would
exist at all, of course.

> 2017-03-21  Kelvin Nilsen  
> 
>   * config/rs6000/rs6000.c (rs6000_option_override_internal): Change
>   handling of certain combinations of target options, including the
>   combinations -mpower8-vector vs. -mno-vsx, -mpower8-vector vs.
>   -mno-power8-vector, and -mpower9_dform vs. -mno-power9-vector.

Those other changes are independent?


Segher

[PATCH,rs6000] Handle conflicting target options -mno-power9-vector and -mcpu=power9

2017-03-22 Thread Kelvin Nilsen 

Internal testing recently revealed that use of the -mno-power9-vector
target option in combination with the -mcpu=power9 target option
results in termination of gcc with the error message:

  power9-dform requires power9-vector

This same problem is seen if the -mno-power9-vector target option is
specified to a gcc which was built using --with-cpu=power9 as an
argument to configure.

In both cases, the preferred behavior is that the target option
-mno-power9-vector causes power9-dform to be automatically disabled.
 This patch implements the preferred behavior and adds a test case to
demonstrate the fix.

The patch has been bootstrapped and tested with no regressions on both
powerpc64-unknown-linux-gnu and powerpc64le-unknown-linux-gnu.  Is this
ok for the trunk?

gcc/testsuite/ChangeLog:

2017-03-21  Kelvin Nilsen  

* gcc.target/powerpc/p9-options-1.c: New test.

gcc/ChangeLog:

2017-03-21  Kelvin Nilsen  

* config/rs6000/rs6000.c (rs6000_option_override_internal): Change
handling of certain combinations of target options, including the
combinations -mpower8-vector vs. -mno-vsx, -mpower8-vector vs.
-mno-power8-vector, and -mpower9_dform vs. -mno-power9-vector.

Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c  (revision 246212)
+++ gcc/config/rs6000/rs6000.c  (working copy)
@@ -4246,9 +4246,22 @@ rs6000_option_override_internal (bool global_init_
 
   if (TARGET_P8_VECTOR && !TARGET_VSX)
 {
-  if (rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR)
+  if ((rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR)
+ && (rs6000_isa_flags_explicit & OPTION_MASK_VSX))
error ("-mpower8-vector requires -mvsx");
-  rs6000_isa_flags &= ~OPTION_MASK_P8_VECTOR;
+  else if ((rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR) == 0)
+   {
+ rs6000_isa_flags &= ~OPTION_MASK_P8_VECTOR;
+ if (rs6000_isa_flags_explicit & OPTION_MASK_VSX)
+   rs6000_isa_flags_explicit |= OPTION_MASK_P8_VECTOR;
+   }
+  else
+   {
+ /* OPTION_MASK_P8_VECTOR is explicit, and OPTION_MASK_VSX is
+not explicit.  */
+ rs6000_isa_flags |= OPTION_MASK_VSX;
+ rs6000_isa_flags_explicit |= OPTION_MASK_VSX;
+   }
 }
 
   if (TARGET_VSX_TIMODE && !TARGET_VSX)
@@ -4448,9 +4461,22 @@ rs6000_option_override_internal (bool global_init_
 error messages.  However, if users have managed to select
 power9-vector without selecting power8-vector, they
 already know about undocumented flags.  */
-  if (rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR)
+  if ((rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR) &&
+ (rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR))
error ("-mpower9-vector requires -mpower8-vector");
-  rs6000_isa_flags &= ~OPTION_MASK_P9_VECTOR;
+  else if ((rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR) == 0)
+   {
+ rs6000_isa_flags &= ~OPTION_MASK_P9_VECTOR;
+ if (rs6000_isa_flags_explicit & OPTION_MASK_P8_VECTOR)
+   rs6000_isa_flags_explicit |= OPTION_MASK_P9_VECTOR;
+   }
+  else
+   {
+ /* OPTION_MASK_P9_VECTOR is explicit and
+OPTION_MASK_P8_VECTOR is not explicit.  */
+ rs6000_isa_flags |= OPTION_MASK_P8_VECTOR;
+ rs6000_isa_flags_explicit |= OPTION_MASK_P8_VECTOR;
+   }
 }
 
   /* -mpower9-dform turns on both -mpower9-dform-scalar and
@@ -4479,10 +4505,25 @@ rs6000_option_override_internal (bool global_init_
 error messages.  However, if users have managed to select
 power9-dform without selecting power9-vector, they
 already know about undocumented flags.  */
-  if (rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR)
+  if ((rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR)
+ && (rs6000_isa_flags_explicit & (OPTION_MASK_P9_DFORM_SCALAR
+  | OPTION_MASK_P9_DFORM_VECTOR)))
error ("-mpower9-dform requires -mpower9-vector");
-  rs6000_isa_flags &= ~(OPTION_MASK_P9_DFORM_SCALAR
-   | OPTION_MASK_P9_DFORM_VECTOR);
+  else if (rs6000_isa_flags_explicit & OPTION_MASK_P9_VECTOR)
+   {
+ rs6000_isa_flags &=
+   ~(OPTION_MASK_P9_DFORM_SCALAR | OPTION_MASK_P9_DFORM_VECTOR);
+ rs6000_isa_flags_explicit |=
+   (OPTION_MASK_P9_DFORM_SCALAR | OPTION_MASK_P9_DFORM_VECTOR);
+   }
+  else
+   {
+ /* We know that OPTION_MASK_P9_VECTOR is not explicit and
+OPTION_MASK_P9_DFORM_SCALAR or OPTION_MASK_P9_DORM_VECTOR
+may be explicit.  */
+ rs6000_isa_flags |= OPTION_MASK_P9_VECTOR;
+ rs6000_isa_flags_explicit |= OPTION_MASK_P9_VECTOR;
+   }
 }
 
   if (TARGET_P9_DFORM_SCALAR && !TARGET_UPPER_REGS_DF)
Index: