Patchwork RFC, start to add attribute target support to powerpc

login
register
mail settings
Submitter Michael Meissner
Date Oct. 8, 2010, 12:20 a.m.
Message ID <20101008002011.GA24300@hungry-tiger.westford.ibm.com>
Download mbox | patch
Permalink /patch/67120/
State New
Headers show

Comments

Michael Meissner - Oct. 8, 2010, 12:20 a.m.
On Thu, Oct 07, 2010 at 11:26:10PM +0000, Joseph S. Myers wrote:
> On Thu, 7 Oct 2010, Michael Meissner wrote:
> 
> > > * You should not have a global variable of that structure type.  You 
> > > should have a field in the gcc_options structure with that type instead.  
> > > Putting state in gcc_options is preferred to having it in separate 
> > > globals; only that way will it be possible to use that state as part of 
> > > multilib selection, because only the state in gcc_options will exist for 
> > > each multilib when the comparison of gcc_options structures, instead of 
> > > textual matching of options, becomes how multilib selection is done as 
> > > described at <http://gcc.gnu.org/ml/gcc/2010-01/msg00063.html>.
> > 
> > However, the target specific stuff has nothing to do with multilib, since on
> > power, we don't have multilibs for each cpu type on the linux side of things.
> > The distros tend to only want 32-bit and 64-bit multilibs.  On the eabi side of
> > things, I recall there were more multilibs for the different options.
> 
> Multilibs are used just as much for embedded GNU/Linux on Power as for 
> other architectures, so this is just as relevant there - for example, 
> given a toolchain with both SPE and non-SPE multilibs, to select the right 
> multilib for the features enabled whatever combination of options is used 
> to enable those features.  (Thus, having the flag for SPE in the 
> gcc_options structure is desirable; it could well be used for selecting a 
> multilib.)

The whole point of the target attribute is so that a single executable can have
support for different similar architectures.  I hope eventually to tie this in
with the binutils shared library relocation STT_IFUNC that will allow the
shared library loader to pick which is the fastest version of the library for
the particular hardware you are running on.


I imagine the code would be something like at least initially:

	if (have_power7)
	   p7_func ();
	else if (have_power6)
	   p6_func ();
	else
	   p5_func ();

Multilibs on the other hand for each different cpu you build a new executable.
Sometimes that is appropriate, and sometimes having only a single binary is
appropriate.

I looked at the options stuff, and figured we needed a combination of Variable
and TargetSave, so I added TargetVariable.  It would be nice if we could move
the x86 to use TargetVariable instead of TargetSave, and delete TargetSave.

I coded the following today to add TargetVariable.  At the present, I want to
play around with it more, and change my current patch to use that instead of
making all of the invasive changes, but in case you were thinking about the
same thing, I figured maybe eliminate extra work.

2010-10-07  Michael Meissner  <meissner@linux.vnet.ibm.com>

	* optc-gen.awk: Add support for a record type, TargetVariable that
	combines Variable and TargetSave.
	* opth-gen.awk: Ditto.
	* doc/options.texi: Ditto.
Joseph S. Myers - Oct. 8, 2010, 12:27 a.m.
On Thu, 7 Oct 2010, Michael Meissner wrote:

> I looked at the options stuff, and figured we needed a combination of Variable
> and TargetSave, so I added TargetVariable.  It would be nice if we could move
> the x86 to use TargetVariable instead of TargetSave, and delete TargetSave.

Yes, this looks like a good way of doing things that fits in well with the 
general options infrastructure.  Thanks for working out how to integrate 
this with .opt files and gcc_options.

>  @item
> +A variable record to define a variable used to store option
> +information.  These records have two fields: the string
> +@samp{TargetVariable}, and a declaration of the type and name of the
> +variable, optionally with an initializer (but without any trailing
> +@samp{;}).  @samp{TargetVariable} are a combination of @samp{Variable}
> +and @samp{TargetSave} records in that the variable is defined in the
> +@code{gcc_options} structure, but these variables are also stored in
> +@code{cl_target_option} structure.  The variables are saved in the
> +target save code and restored in the target restore code.
> +
> +records have two fields: the string @samp{TargetSave}, and a
> +declaration type to go in the @code{cl_target_option} structure.

A cut-and-paste problem here with the second partial paragraph?
Michael Meissner - Oct. 8, 2010, 12:34 a.m.
On Fri, Oct 08, 2010 at 12:27:18AM +0000, Joseph S. Myers wrote:
> On Thu, 7 Oct 2010, Michael Meissner wrote:
> 
> > I looked at the options stuff, and figured we needed a combination of Variable
> > and TargetSave, so I added TargetVariable.  It would be nice if we could move
> > the x86 to use TargetVariable instead of TargetSave, and delete TargetSave.
> 
> Yes, this looks like a good way of doing things that fits in well with the 
> general options infrastructure.  Thanks for working out how to integrate 
> this with .opt files and gcc_options.
> 
> >  @item
> > +A variable record to define a variable used to store option
> > +information.  These records have two fields: the string
> > +@samp{TargetVariable}, and a declaration of the type and name of the
> > +variable, optionally with an initializer (but without any trailing
> > +@samp{;}).  @samp{TargetVariable} are a combination of @samp{Variable}
> > +and @samp{TargetSave} records in that the variable is defined in the
> > +@code{gcc_options} structure, but these variables are also stored in
> > +@code{cl_target_option} structure.  The variables are saved in the
> > +target save code and restored in the target restore code.
> > +
> > +records have two fields: the string @samp{TargetSave}, and a
> > +declaration type to go in the @code{cl_target_option} structure.
> 
> A cut-and-paste problem here with the second partial paragraph?

Probably.  I shortened the buffer when posting, and I may have shorted it too
much.

Enum's are assumed to be stored as ints, due to the fact that the generator
file doesn't have the include files present.

Patch

Index: gcc/optc-gen.awk
===================================================================
--- gcc/optc-gen.awk	(revision 165044)
+++ gcc/optc-gen.awk	(working copy)
@@ -31,6 +31,7 @@  BEGIN {
 	n_langs = 0
 	n_target_save = 0
 	n_extra_vars = 0
+	n_extra_target_vars = 0
         quote = "\042"
 	comma = ","
 	FS=SUBSEP
@@ -53,6 +54,24 @@  BEGIN {
 			extra_vars[n_extra_vars] = $2
 			n_extra_vars++
 		}
+		else if ($1 == "TargetVariable") {
+			# Combination of TargetSave and Variable
+			extra_vars[n_extra_vars] = $2
+			n_extra_vars++
+
+			var = $2
+			sub(" *=.*", "", var)
+			orig_var = var
+			name = var
+			type = var
+			sub("^.*[ *]", "", name)
+			sub(" *" name "$", "", type)
+			target_save_decl[n_target_save] = type " x_" name
+			n_target_save++
+
+			extra_target_vars[n_extra_target_vars] = name
+			n_extra_target_vars++;
+		}
 		else {
 			name = opt_args("Mask", $1)
 			if (name == "") {
@@ -83,6 +102,9 @@  print "#endif /* GCC_DRIVER */"
 print ""
 
 have_save = 0;
+if (n_extra_target_vars)
+	have_save = 1
+
 print "struct gcc_options global_options =\n{"
 for (i = 0; i < n_extra_vars; i++) {
 	var = extra_vars[i]
@@ -303,6 +325,7 @@  print "{";
 n_opt_char = 2;
 n_opt_short = 0;
 n_opt_int = 0;
+n_opt_enum = 0;
 n_opt_other = 0;
 var_opt_char[0] = "optimize";
 var_opt_char[1] = "optimize_size";
@@ -329,6 +352,9 @@  for (i = 0; i < n_opts; i++) {
 		else if (otype ~ "^((un)?signed +)?short *$")
 			var_opt_short[n_opt_short++] = name;
 
+		else if (otype ~ "^enum +[a-zA-Z0-9_]+ *")
+			var_opt_enum[n_opt_enum++] = name;
+
 		else if (otype ~ "^((un)?signed +)?char *$") {
 			var_opt_char[n_opt_char++] = name;
 			if (otype ~ "^unsigned +char *$")
@@ -352,6 +378,10 @@  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] ";";
 }
@@ -376,6 +406,10 @@  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] ";";
 }
@@ -409,6 +443,15 @@  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\",";
@@ -447,6 +490,7 @@  print "{";
 n_target_char = 0;
 n_target_short = 0;
 n_target_int = 0;
+n_target_enum = 0;
 n_target_other = 0;
 
 if (have_save) {
@@ -467,6 +511,9 @@  if (have_save) {
 			else if (otype ~ "^((un)?signed +)?short *$")
 				var_target_short[n_target_short++] = name;
 
+			else if (otype ~ "^enum +[_a-zA-Z0-9]+ *$")
+				var_target_enum[n_target_enum++] = name;
+
 			else if (otype ~ "^((un)?signed +)?char *$") {
 				var_target_char[n_target_char++] = name;
 				if (otype ~ "^unsigned +char *$")
@@ -498,10 +545,18 @@  print "  if (targetm.target_option.save)
 print "    targetm.target_option.save (ptr);";
 print "";
 
+for (i = 0; i < n_extra_target_vars; i++) {
+	print "  ptr->x_" extra_target_vars[i] " = opts->x_" extra_target_vars[i] ";";
+}
+
 for (i = 0; i < n_target_other; i++) {
 	print "  ptr->x_" var_target_other[i] " = opts->x_" var_target_other[i] ";";
 }
 
+for (i = 0; i < n_target_enum; i++) {
+	print "  ptr->x_" var_target_enum[i] " = opts->x_" var_target_enum[i] ";";
+}
+
 for (i = 0; i < n_target_int; i++) {
 	print "  ptr->x_" var_target_int[i] " = opts->x_" var_target_int[i] ";";
 }
@@ -522,10 +577,18 @@  print "void";
 print "cl_target_option_restore (struct gcc_options *opts, struct cl_target_option *ptr)";
 print "{";
 
+for (i = 0; i < n_extra_target_vars; i++) {
+	print "  opts->x_" extra_target_vars[i] " = ptr->x_" extra_target_vars[i] ";";
+}
+
 for (i = 0; i < n_target_other; i++) {
 	print "  opts->x_" var_target_other[i] " = ptr->x_" var_target_other[i] ";";
 }
 
+for (i = 0; i < n_target_enum; i++) {
+	print "  opts->x_" var_target_enum[i] " = ptr->x_" var_target_enum[i] ";";
+}
+
 for (i = 0; i < n_target_int; i++) {
 	print "  opts->x_" var_target_int[i] " = ptr->x_" var_target_int[i] ";";
 }
@@ -564,6 +627,15 @@  for (i = 0; i < n_target_other; i++) {
 	print "";
 }
 
+for (i = 0; i < n_target_enum; i++) {
+	print "  if (ptr->x_" var_target_enum[i] ")";
+	print "    fprintf (file, \"%*s%s (%#x)\\n\",";
+	print "             indent, \"\",";
+	print "             \"" var_target_enum[i] "\",";
+	print "             ptr->x_" var_target_enum[i] ");";
+	print "";
+}
+
 for (i = 0; i < n_target_int; i++) {
 	print "  if (ptr->x_" var_target_int[i] ")";
 	print "    fprintf (file, \"%*s%s (%#x)\\n\",";
Index: gcc/opth-gen.awk
===================================================================
--- gcc/opth-gen.awk	(revision 165044)
+++ gcc/opth-gen.awk	(working copy)
@@ -29,6 +29,7 @@  BEGIN {
 	n_langs = 0
 	n_target_save = 0
 	n_extra_vars = 0
+	n_extra_target_vars = 0
 	n_extra_masks = 0
 	FS=SUBSEP
 }
@@ -48,6 +49,24 @@  BEGIN {
 			extra_vars[n_extra_vars] = $2
 			n_extra_vars++
 		}
+		else if ($1 == "TargetVariable") {
+			# Combination of TargetSave and Variable
+			extra_vars[n_extra_vars] = $2
+			n_extra_vars++
+
+			var = $2
+			sub(" *=.*", "", var)
+			orig_var = var
+			name = var
+			type = var
+			sub("^.*[ *]", "", name)
+			sub(" *" name "$", "", type)
+			target_save_decl[n_target_save] = type " x_" name
+			n_target_save++
+
+			extra_target_vars[n_extra_target_vars] = name
+			n_extra_target_vars++;
+		}
 		else {
 			name = opt_args("Mask", $1)
 			if (name == "") {
@@ -72,6 +91,8 @@  print "#define OPTIONS_H"
 print ""
 
 have_save = 0;
+if (n_extra_target_vars)
+	have_save = 1
 
 print "#ifndef GENERATOR_FILE"
 print "struct gcc_options\n{"
@@ -87,10 +108,16 @@  for (i = 0; i < n_extra_vars; i++) {
 	sub(" *" name "$", "", type)
 	var_seen[name] = 1
 	print "#ifdef GENERATOR_FILE"
+	sub("^enum +[a-zA-Z0-9_]+ +", "int ", orig_var)
 	print "extern " orig_var ";"
 	print "#else"
-	print "  " type " x_" name ";"
-	print "#define " name " global_options.x_" name
+	if (type ~ "^enum +[_a-zA-Z0-9]+ *$") {
+		print "  int x_" name ";"
+		print "#define " name " ((" type ")global_options.x_" name ")"
+	} else {
+		print "  " type " x_" name ";"
+		print "#define " name " global_options.x_" name
+	}
 	print "#endif"
 }
 
@@ -146,6 +173,7 @@  print "{";
 n_opt_char = 2;
 n_opt_short = 0;
 n_opt_int = 0;
+n_opt_enum = 0;
 n_opt_other = 0;
 var_opt_char[0] = "unsigned char x_optimize";
 var_opt_char[1] = "unsigned char x_optimize_size";
@@ -170,6 +198,9 @@  for (i = 0; i < n_opts; i++) {
 		else if (otype ~ "^((un)?signed +)?char *$")
 			var_opt_char[n_opt_char++] = otype "x_" name;
 
+		else if (otype ~ "^enum +[a-zA-Z0-9_]+ *$")
+			var_opt_enum[n_opt_enum++] = "int x_" name;
+
 		else
 			var_opt_other[n_opt_other++] = otype "x_" name;
 	}
@@ -179,6 +210,10 @@  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] ";";
 }
@@ -202,6 +237,7 @@  print "{";
 n_target_char = 0;
 n_target_short = 0;
 n_target_int = 0;
+n_target_enum = 0;
 n_target_other = 0;
 
 for (i = 0; i < n_target_save; i++) {
@@ -214,6 +250,11 @@  for (i = 0; i < n_target_save; i++) {
 	else if (target_save_decl[i] ~ "^((un)?signed +)?char +[_a-zA-Z0-9]+$")
 		var_target_char[n_target_char++] = target_save_decl[i];
 
+	else if (target_save_decl[i] ~ "^enum +[_a-zA-Z0-9]+ +[_a-zA-Z0-9]+$") {
+		var = target_save_decl[i]
+		sub("^enum +[_a-zA-Z0-9]+ +", "int", var)
+		var_target_enum[n_target_enum++] = var
+	}
 	else
 		var_target_other[n_target_other++] = target_save_decl[i];
 }
@@ -239,6 +280,9 @@  if (have_save) {
 			else if (otype ~ "^((un)?signed +)?char *$")
 				var_target_char[n_target_char++] = otype "x_" name;
 
+			else if (otype ~ "^enum +[_a-zA-Z0-9]+ +[_a-zA-Z0-9]+")
+				var_target_enum[n_target_enum++] = "int x_" name;
+
 			else
 				var_target_other[n_target_other++] = otype "x_" name;
 		}
@@ -251,6 +295,10 @@  for (i = 0; i < n_target_other; i++) {
 	print "  " var_target_other[i] ";";
 }
 
+for (i = 0; i < n_target_enum; i++) {
+	print "  " var_target_int[i] ";";
+}
+
 for (i = 0; i < n_target_int; i++) {
 	print "  " var_target_int[i] ";";
 }
Index: gcc/doc/options.texi
===================================================================
--- gcc/doc/options.texi	(revision 165044)
+++ gcc/doc/options.texi	(working copy)
@@ -52,6 +52,20 @@  for variables set in option handlers rat
 @code{Var} properties.
 
 @item
+A variable record to define a variable used to store option
+information.  These records have two fields: the string
+@samp{TargetVariable}, and a declaration of the type and name of the
+variable, optionally with an initializer (but without any trailing
+@samp{;}).  @samp{TargetVariable} are a combination of @samp{Variable}
+and @samp{TargetSave} records in that the variable is defined in the
+@code{gcc_options} structure, but these variables are also stored in
+@code{cl_target_option} structure.  The variables are saved in the
+target save code and restored in the target restore code.
+
+records have two fields: the string @samp{TargetSave}, and a
+declaration type to go in the @code{cl_target_option} structure.
+
+@item
 An option definition record.  These records have the following fields:
 @enumerate
 @item