From patchwork Wed Nov 17 17:21:12 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Meissner X-Patchwork-Id: 71605 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 5CEEAB71B7 for ; Thu, 18 Nov 2010 04:21:32 +1100 (EST) Received: (qmail 32082 invoked by alias); 17 Nov 2010 17:21:29 -0000 Received: (qmail 32064 invoked by uid 22791); 17 Nov 2010 17:21:26 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL, BAYES_00, NO_DNS_FOR_FROM, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e39.co.us.ibm.com (HELO e39.co.us.ibm.com) (32.97.110.160) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 17 Nov 2010 17:21:20 +0000 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e39.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id oAHH9wQ8008849 for ; Wed, 17 Nov 2010 10:09:58 -0700 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oAHHLFUV196766 for ; Wed, 17 Nov 2010 10:21:15 -0700 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oAHHLEAk006689 for ; Wed, 17 Nov 2010 10:21:14 -0700 Received: from hungry-tiger.westford.ibm.com (dyn9033037049.westford.ibm.com [9.33.37.49]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id oAHHLEF1006659; Wed, 17 Nov 2010 10:21:14 -0700 Received: by hungry-tiger.westford.ibm.com (Postfix, from userid 500) id 1365CF7F91; Wed, 17 Nov 2010 12:21:12 -0500 (EST) Date: Wed, 17 Nov 2010 12:21:12 -0500 From: Michael Meissner To: "Joseph S. Myers" Cc: Michael Meissner , gcc-patches@gcc.gnu.org, dje.gcc@gmail.com Subject: Re: [PATCH] Add attribute((target("..."))) and pragma target to PowerPC Message-ID: <20101117172112.GA4915@hungry-tiger.westford.ibm.com> Mail-Followup-To: Michael Meissner , "Joseph S. Myers" , gcc-patches@gcc.gnu.org, dje.gcc@gmail.com References: <20101031182504.GA14215@hungry-tiger.westford.ibm.com> <20101101174906.GA8243@hungry-tiger.westford.ibm.com> <20101101231522.GA22461@hungry-tiger.westford.ibm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Mon, Nov 01, 2010 at 11:52:37PM +0000, Joseph S. Myers wrote: > On Mon, 1 Nov 2010, Michael Meissner wrote: > > > On Mon, Nov 01, 2010 at 01:49:06PM -0400, Michael Meissner wrote: > > > The simplest way to fix this is to require the user on 32-bit to use an > > > explicit -maltivec-abi switch if they want to switch to a power7 cpu or enable > > > altivec (similarly for the rs6000_spe_abi, rs6000_float_gprs, and > > > rs6000_darwin64_abi variables). > > > > Whoops, that should be -mabi=altivec. > > > > The following patch applied on top of my previous patch, will give an error if > > any option changes the various abi's. It also changes the debug information > > somewhat. Does this answer your objection? > > It looks like it. Note that there are problems with diagnostic > formatting: > > > + error ("You cannot change long double size within a target attribute " > > + "or pragma"); > > Diagnostics should not start with a capital letter, and "You cannot" is > not the normal style; I'd think the message should be something like > > error ("target attribute or pragma changes long double size"); > > or use sorry () if you think the feature should be supported (that's more > likely with the AltiVec ABI, though maybe that should be a separate > attribute instead) and the lack of support is a limitation of the > implementation. > > > + error ("You cannot switch to the altivec abi within a target " > > "AltiVec" and "ABI". Likewise elsewhere. This patch fixes the error messages in rs6000.c to address the concerns from Joesph Myers. In addition, as we discussed on irc, I made the variable flag_fp_contract_mode saved with the optimization options, so that if a backend decides to change the FMA mode, it will be automatically swapped. Originally I was going to add more infrastructure to declare such variables as optimization variables, but I decided for just one more variable, it was simpler to just add it in, rather than come up with new infrastructure. While I was there, I changed the ordering within the structure so that enums come between ints and shorts. I also fixed an obvious typo I made originally with these patches. Assuming David gives me the ok to check in the powerpc side of things, are the awk script changes ok for the machine independent changes? 2010-11-17 Michael Meissner * config/rs6000/rs6000.c (rs6000_option_override_internal): Make error messages for target attributes or pragmas conform to existing usage. (rs6000_inner_target_options): Ditto. * opth-gen.awk: Add flag_fp_contract_mode to variables saved as optimization variables. Sort enums between ints and shorts. Fix cut+paste error in restoration of enum optimize variables. Cast enum values to int before printing. * optc-gen.awk: Ditto. Index: gcc/optc-gen.awk =================================================================== --- gcc/optc-gen.awk (revision 166816) +++ gcc/optc-gen.awk (working copy) @@ -341,12 +341,13 @@ print "{"; n_opt_char = 2; n_opt_short = 0; n_opt_int = 0; -n_opt_enum = 0; +n_opt_enum = 1; n_opt_other = 0; var_opt_char[0] = "optimize"; var_opt_char[1] = "optimize_size"; var_opt_range["optimize"] = "0, 255"; var_opt_range["optimize_size"] = "0, 255"; +var_opt_enum[0] = "flag_fp_contract_mode"; # Sort by size to mimic how the structure is laid out to be friendlier to the # cache. @@ -394,14 +395,14 @@ for (i = 0; i < n_opt_other; i++) { print " ptr->x_" var_opt_other[i] " = opts->x_" var_opt_other[i] ";"; } -for (i = 0; i < n_opt_enum; i++) { - print " ptr->x_" var_opt_enum[i] " = opts->x_" var_opt_enum[i] ";"; -} - for (i = 0; i < n_opt_int; i++) { print " ptr->x_" var_opt_int[i] " = opts->x_" var_opt_int[i] ";"; } +for (i = 0; i < n_opt_enum; i++) { + print " ptr->x_" var_opt_enum[i] " = opts->x_" var_opt_enum[i] ";"; +} + for (i = 0; i < n_opt_short; i++) { print " ptr->x_" var_opt_short[i] " = opts->x_" var_opt_short[i] ";"; } @@ -422,14 +423,14 @@ for (i = 0; i < n_opt_other; i++) { print " opts->x_" var_opt_other[i] " = ptr->x_" var_opt_other[i] ";"; } -for (i = 0; i < n_opt_enum; i++) { - print " ptr->x_" var_opt_enum[i] " = opts->x_" var_opt_enum[i] ";"; -} - for (i = 0; i < n_opt_int; i++) { print " opts->x_" var_opt_int[i] " = ptr->x_" var_opt_int[i] ";"; } +for (i = 0; i < n_opt_enum; i++) { + print " opts->x_" var_opt_enum[i] " = ptr->x_" var_opt_enum[i] ";"; +} + for (i = 0; i < n_opt_short; i++) { print " opts->x_" var_opt_short[i] " = ptr->x_" var_opt_short[i] ";"; } @@ -459,15 +460,6 @@ for (i = 0; i < n_opt_other; i++) { print ""; } -for (i = 0; i < n_opt_enum; i++) { - print " if (ptr->x_" var_opt_enum[i] ")"; - print " fprintf (file, \"%*s%s (%#x)\\n\","; - print " indent_to, \"\","; - print " \"" var_opt_enum[i] "\","; - print " ptr->x_" var_opt_enum[i] ");"; - print ""; -} - for (i = 0; i < n_opt_int; i++) { print " if (ptr->x_" var_opt_int[i] ")"; print " fprintf (file, \"%*s%s (%#x)\\n\","; @@ -477,6 +469,14 @@ for (i = 0; i < n_opt_int; i++) { print ""; } +for (i = 0; i < n_opt_enum; i++) { + print " fprintf (file, \"%*s%s (%#x)\\n\","; + print " indent_to, \"\","; + print " \"" var_opt_enum[i] "\","; + print " (int) ptr->x_" var_opt_enum[i] ");"; + print ""; +} + for (i = 0; i < n_opt_short; i++) { print " if (ptr->x_" var_opt_short[i] ")"; print " fprintf (file, \"%*s%s (%#x)\\n\","; Index: gcc/opth-gen.awk =================================================================== --- gcc/opth-gen.awk (revision 166816) +++ gcc/opth-gen.awk (working copy) @@ -190,10 +190,11 @@ print "{"; n_opt_char = 2; n_opt_short = 0; n_opt_int = 0; -n_opt_enum = 0; +n_opt_enum = 1; n_opt_other = 0; var_opt_char[0] = "unsigned char x_optimize"; var_opt_char[1] = "unsigned char x_optimize_size"; +var_opt_enum[0] = "enum fp_contract_mode x_flag_fp_contract_mode"; for (i = 0; i < n_opts; i++) { if (flag_set_p("Optimization", flags[i])) { @@ -227,14 +228,14 @@ for (i = 0; i < n_opt_other; i++) { print " " var_opt_other[i] ";"; } -for (i = 0; i < n_opt_enum; i++) { - print " " var_opt_enum[i] ";"; -} - for (i = 0; i < n_opt_int; i++) { print " " var_opt_int[i] ";"; } +for (i = 0; i < n_opt_enum; i++) { + print " " var_opt_enum[i] ";"; +} + for (i = 0; i < n_opt_short; i++) { print " " var_opt_short[i] ";"; } Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 166818) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -2889,8 +2889,7 @@ rs6000_option_override_internal (bool gl if (main_target_opt != NULL && (main_target_opt->x_rs6000_long_double_type_size != RS6000_DEFAULT_LONG_DOUBLE_SIZE)) - error ("You cannot change long double size within a target attribute " - "or pragma"); + error ("target attribute or pragma changes long double size"); else rs6000_long_double_type_size = RS6000_DEFAULT_LONG_DOUBLE_SIZE; } @@ -2911,8 +2910,7 @@ rs6000_option_override_internal (bool gl if (TARGET_XCOFF && (TARGET_ALTIVEC || TARGET_VSX)) { if (main_target_opt != NULL && !main_target_opt->x_rs6000_altivec_abi) - error ("You cannot switch to the altivec abi within a target " - "attribute or pragma"); + error ("target attribute or pragma changes AltiVec ABI"); else rs6000_altivec_abi = 1; } @@ -2927,8 +2925,7 @@ rs6000_option_override_internal (bool gl { if (main_target_opt != NULL && !main_target_opt->x_rs6000_altivec_abi) - error ("You cannot switch to the altivec abi within a target " - "attribute or pragma"); + error ("target attribute or pragma changes AltiVec ABI"); else rs6000_altivec_abi = 1; } @@ -2945,8 +2942,7 @@ rs6000_option_override_internal (bool gl && TARGET_64BIT) { if (main_target_opt != NULL && !main_target_opt->x_rs6000_darwin64_abi) - error ("You cannot switch to the darwin64 abi within a target " - "attribute or pragma"); + error ("target attribute or pragma changes darwin64 ABI"); else { rs6000_darwin64_abi = 1; @@ -2987,8 +2983,7 @@ rs6000_option_override_internal (bool gl && ((main_target_opt->x_rs6000_spe_abi != rs6000_spe_abi) || (main_target_opt->x_rs6000_spe != rs6000_spe) || (main_target_opt->x_rs6000_float_gprs != rs6000_float_gprs))) - error ("You cannot switch to the SPE abi within a target " - "attribute or pragma"); + error ("target attribute or pragma changes SPE ABI"); else { if (!rs6000_explicit_options.spe_abi) @@ -3309,11 +3304,11 @@ rs6000_option_override_internal (bool gl if (main_target_opt) { if (main_target_opt->x_rs6000_single_float != rs6000_single_float) - error ("You are not allowed to change the single precision floating " - "point unit within a target attribute or pragma"); + error ("target attribute or pragma changes single precision floating " + "point"); if (main_target_opt->x_rs6000_double_float != rs6000_double_float) - error ("You are not allowed to change the double precision floating " - "point unit within a target attribute or pragma"); + error ("target attribute or pragma changes double precision floating " + "point"); } /* If not explicitly specified via option, decide whether to generate indexed @@ -27547,7 +27542,7 @@ rs6000_inner_target_options (tree args, } if (cpu_opt) - error ("Invalid cpu \"%s\" for %s\"%s\"%s", cpu_opt, eprefix, + error ("invalid cpu \"%s\" for %s\"%s\"%s", cpu_opt, eprefix, q, esuffix); else if (not_valid_p) error ("%s\"%s\"%s is not allowed", eprefix, q, esuffix);