Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
On Tue, Oct 9, 2012 at 6:58 PM, Michael Meissner wrote: > Ok, David preferred the 2 series of patches which replace all of the flags in > target_flags to rs6000_isa_flags to the 3 series of patches, which started > over, and added a new flag word, but did not change the existing options. > > In an effort to simplify the main patch, I'm going to push out some of the > patches that are standalone. This patch fixes the 3 signed/unsigned warnings > that were caused by comparing an integer type with an enumeration. I did > bootstap and make check with no regressions. Is it ok to install (it is > probably ok under the obvious rule)? > > 2012-10-09 Michael Meissner > > * config/rs6000/rs6000.c (altivec_expand_dst_builtin): Fix signed > vs. unsigned warnings by using enum type for function code. > (paired_expand_builtin): Likewise. > (spe_expand_builtin): Likewise. This patch is okay. Thanks, David
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
Ok, David preferred the 2 series of patches which replace all of the flags in
target_flags to rs6000_isa_flags to the 3 series of patches, which started
over, and added a new flag word, but did not change the existing options.
In an effort to simplify the main patch, I'm going to push out some of the
patches that are standalone. This patch fixes the 3 signed/unsigned warnings
that were caused by comparing an integer type with an enumeration. I did
bootstap and make check with no regressions. Is it ok to install (it is
probably ok under the obvious rule)?
2012-10-09 Michael Meissner
* config/rs6000/rs6000.c (altivec_expand_dst_builtin): Fix signed
vs. unsigned warnings by using enum type for function code.
(paired_expand_builtin): Likewise.
(spe_expand_builtin): Likewise.
--
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 192265)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -10442,7 +10442,7 @@ altivec_expand_dst_builtin (tree exp, rt
bool *expandedp)
{
tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
- unsigned int fcode = DECL_FUNCTION_CODE (fndecl);
+ enum rs6000_builtins fcode = (enum rs6000_builtins) DECL_FUNCTION_CODE
(fndecl);
tree arg0, arg1, arg2;
enum machine_mode mode0, mode1;
rtx pat, op0, op1, op2;
@@ -10844,7 +10844,7 @@ static rtx
paired_expand_builtin (tree exp, rtx target, bool * expandedp)
{
tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
- unsigned int fcode = DECL_FUNCTION_CODE (fndecl);
+ enum rs6000_builtins fcode = (enum rs6000_builtins) DECL_FUNCTION_CODE
(fndecl);
const struct builtin_description *d;
size_t i;
@@ -10909,7 +10909,7 @@ spe_expand_builtin (tree exp, rtx target
{
tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0);
tree arg1, arg0;
- unsigned int fcode = DECL_FUNCTION_CODE (fndecl);
+ enum rs6000_builtins fcode = (enum rs6000_builtins) DECL_FUNCTION_CODE
(fndecl);
enum insn_code icode;
enum machine_mode tmode, mode0;
rtx pat, op0;
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. > 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
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. > >>> 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= 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. -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. >>> 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= 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
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. > > 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= 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. -- Michael Meissner, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. > 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 > > * config/rs6000/rs6000.c (rs6000_option_override_internal): If > -mcpu= is not specified and the compiler is not configured > using --with-cpu=, 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=, 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)
Re: [PATCH] Rs6000 infrastructure cleanup (switches), revised patch #2c
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= and the compiler was not configured with --with-cpu=. 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 * config/rs6000/rs6000.c (rs6000_option_override_internal): If -mcpu= is not specified and the compiler is not configured using --with-cpu=, 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=, 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, IBM 5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA meiss...@linux.vnet.ibm.com fax +1 (978) 399-6899
