diff mbox

Add attribute((target("..."))) and pragma target to PowerPC

Message ID 20101117172112.GA4915@hungry-tiger.westford.ibm.com
State New
Headers show

Commit Message

Michael Meissner Nov. 17, 2010, 5:21 p.m. UTC 
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  <meissner@linux.vnet.ibm.com>

	* 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.

Comments

David Edelsohn Nov. 17, 2010, 6:33 p.m. UTC | #1 
On Wed, Nov 17, 2010 at 12:21 PM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> 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  <meissner@linux.vnet.ibm.com>
>
>        * 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.

The rs6000 changes are okay, but I believe that the AIX options were
omitted from the rs6000_opt_masks[] array, so please fix that if
necessary.

Thanks, David
diff mbox

Patch

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);