diff mbox

Rs6000 infrastructure cleanup (switches), revised patch #2c

Message ID 20120928181106.GA28376@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner Sept. 28, 2012, 6:11 p.m. UTC 
Segher Boessenkool asked me on IRC to break out the fix in the last change.
This patch is just the change to set the default options if the user did not
use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
Here are the patches.

I can submit this patch first if David desires, and then resubmit the first of
the infrastructure patches again, or commit both together.

2012-09-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* config/rs6000/rs6000.c (rs6000_option_override_internal): If
	-mcpu=<xxx> is not specified and the compiler is not configured
	using --with-cpu=<xxx>, use the bits from the TARGET_DEFAULT to
	set the initial options.

Comments

Gunther Nikl Oct. 2, 2012, 8:13 a.m. UTC | #1 
Michael Meissner wrote:
> Segher Boessenkool asked me on IRC to break out the fix in the last change.
> This patch is just the change to set the default options if the user did not
> use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
> Here are the patches.

Which GCC releases are affected by this bug?

Regards,
Gunther

> I can submit this patch first if David desires, and then resubmit the first of
> the infrastructure patches again, or commit both together.
> 
> 2012-09-28  Michael Meissner  <meissner@linux.vnet.ibm.com>
> 
> 	* config/rs6000/rs6000.c (rs6000_option_override_internal): If
> 	-mcpu=<xxx> is not specified and the compiler is not configured
> 	using --with-cpu=<xxx>, use the bits from the TARGET_DEFAULT to
> 	set the initial options.
> 
> Index: gcc/config/rs6000/rs6000.c
> ===================================================================
> --- gcc/config/rs6000/rs6000.c	(revision 191831)
> +++ gcc/config/rs6000/rs6000.c	(working copy)
> @@ -2461,6 +2461,11 @@ rs6000_option_override_internal (bool gl
>    target_flags |= (processor_target_table[cpu_index].target_enable
>  		   & set_masks);
>  
> +  /* If no -mcpu=<xxx>, inherit any default options that were cleared via
> +     POWERPC_MASKS.  */
> +  if (!have_cpu)
> +    target_flags |= (TARGET_DEFAULT & ~target_flags_explicit);
> +
>    if (rs6000_tune_index >= 0)
>      tune_index = rs6000_tune_index;
>    else if (have_cpu)
Michael Meissner Oct. 2, 2012, 4:11 p.m. UTC | #2 
On Tue, Oct 02, 2012 at 10:13:25AM +0200, Gunther Nikl wrote:
> Michael Meissner wrote:
> > Segher Boessenkool asked me on IRC to break out the fix in the last change.
> > This patch is just the change to set the default options if the user did not
> > use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
> > Here are the patches.
> 
> Which GCC releases are affected by this bug?

All of them.  Now, in general users don't see this bug, because distribution
maintainers usually build with an explicit --with-cpu= option, which sets the
default CPU in case the user did not use -mcpu=<xxx> on the command line.  If
neither option was used, the default "powerpc" or "powerpc64" is usually good
enough.

David noticed it when building AIX compilers, because he wanted to add a
default option (-mmfcrf) to the aix*.h definitions to insure that the new get
timebase builtin would generate the correct instructions by default (the
original PowerPCs had a different SPR for the time base than the newer
server machines starting with power4).  He asked me to fix this bug before we
tackle the infrastructure changes.
Gunther Nikl Oct. 4, 2012, 4:33 p.m. UTC | #3 
Michael Meissner schrieb:
> On Tue, Oct 02, 2012 at 10:13:25AM +0200, Gunther Nikl wrote:
>> Michael Meissner wrote:
>>> Segher Boessenkool asked me on IRC to break out the fix in the last change.
>>> This patch is just the change to set the default options if the user did not
>>> use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
>>> Here are the patches.
>> Which GCC releases are affected by this bug?
> 
> All of them.

So this bug is as old as the rs6000 port has PowerPC support? Then GCC
2.95 is also affected?

> Now, in general users don't see this bug, because distribution maintainers
> usually build with an explicit --with-cpu= option, which sets the default
> CPU in case the user did not use -mcpu=<xxx> on the command line.  If neither
> option was used, the default "powerpc" or "powerpc64" is usually good enough.

I am not a distribution user. I have a private PPC port which I always
build without an explicit --with-cpu= option. This option seemed to be
redundant with PROCESSOR_DEFAULT and TARGET_DEFAULT in the target
config file. I will alter my build procedure.

Regards,
Gunther
Michael Meissner Oct. 5, 2012, 6:17 p.m. UTC | #4 
On Thu, Oct 04, 2012 at 06:33:33PM +0200, Gunther Nikl wrote:
> Michael Meissner schrieb:
> > On Tue, Oct 02, 2012 at 10:13:25AM +0200, Gunther Nikl wrote:
> >> Michael Meissner wrote:
> >>> Segher Boessenkool asked me on IRC to break out the fix in the last change.
> >>> This patch is just the change to set the default options if the user did not
> >>> use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
> >>> Here are the patches.
> >> Which GCC releases are affected by this bug?
> > 
> > All of them.
> 
> So this bug is as old as the rs6000 port has PowerPC support? Then GCC
> 2.95 is also affected?
> 
> > Now, in general users don't see this bug, because distribution maintainers
> > usually build with an explicit --with-cpu= option, which sets the default
> > CPU in case the user did not use -mcpu=<xxx> on the command line.  If neither
> > option was used, the default "powerpc" or "powerpc64" is usually good enough.
> 
> I am not a distribution user. I have a private PPC port which I always
> build without an explicit --with-cpu= option. This option seemed to be
> redundant with PROCESSOR_DEFAULT and TARGET_DEFAULT in the target
> config file. I will alter my build procedure.

Well as I said, it is pretty latent, and most people never have noticed it.  It
really depends on what the options are whether you run into the problem.  I
added more verbose debug information to the patches for -mdebug=reg to verify
what options are being set, etc.  Hopefully these patches can finally get
accepted.
Gunther Nikl Oct. 6, 2012, 8:08 p.m. UTC | #5 
Michael Meissner wrote:
> On Thu, Oct 04, 2012 at 06:33:33PM +0200, Gunther Nikl wrote:
>> Michael Meissner schrieb:
>>> On Tue, Oct 02, 2012 at 10:13:25AM +0200, Gunther Nikl wrote:
>>>> Michael Meissner wrote:
>>>>> Segher Boessenkool asked me on IRC to break out the fix in the last change.
>>>>> This patch is just the change to set the default options if the user did not
>>>>> use -mcpu=<xxx> and the compiler was not configured with --with-cpu=<xxx>.
>>>>> Here are the patches.
>>>> Which GCC releases are affected by this bug?
>>> All of them.
>> So this bug is as old as the rs6000 port has PowerPC support? Then GCC
>> 2.95 is also affected?

I took a closer look. AFAICT, the above mentioned bug is a result of
the rewritten option parsing for GCC 4.6. And that was what I wanted
to know. Since this is a regression, should this be fixed in 4.6/4.7?

> Well as I said, it is pretty latent, and most people never have noticed it.
> It really depends on what the options are whether you run into the problem.

Yes, I understood that but this did not address my question.

Regards,
Gunther
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c	(revision 191831)
+++ gcc/config/rs6000/rs6000.c	(working copy)
@@ -2461,6 +2461,11 @@  rs6000_option_override_internal (bool gl
   target_flags |= (processor_target_table[cpu_index].target_enable
 		   & set_masks);
 
+  /* If no -mcpu=<xxx>, inherit any default options that were cleared via
+     POWERPC_MASKS.  */
+  if (!have_cpu)
+    target_flags |= (TARGET_DEFAULT & ~target_flags_explicit);
+
   if (rs6000_tune_index >= 0)
     tune_index = rs6000_tune_index;
   else if (have_cpu)