RFC: Option handling and PR 37565

RFC: Option handling and PR 37565

[Steve Ellcey]

While looking into PR 37565 I began to wonder if we need to modify how
we handle optimization flag handling.  Currently we have
OVERRIDE_OPTIONS, C_COMMON_OVERRIDE_OPTIONS, and OPTIMIZATION_OPTIONS to
set or override the optimization flags a user gives.  One proposal to
fix 37565 was to split OVERRIDE_OPTIONS into OVERRIDE_OPTIONS_ALWAYS and
OVERRIDE_OPTIONS_ONCE  which would create two new macros.

But I was wondering if a cleaner method to deal with these options would
be to get rid of all these macros and use target functions to access
flag values.

My idea is that we would never change the values of the flags that are
set by the user but instead would have a target overridable function
that returns the value for a given flag by default but in which the
return value could be overridden for some flags on some targets.

So instead of 
	if (flag_var_tracking)
we would have
	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))

The default behaviour of get_optimization_flag_value when passed
OPT_fvar_tracking would be to return the value of flag_var_tracking but
if a particular target didn't support this optimization they could have
a target specific version of get_optimization_flag_value that always
returned 0 for OPT_fvar_tracking.  That way we don't have to use
OVERRIDE_OPTIONS to set flag_var_tracking to 0 or worry about it getting
reset by the __optimize__ attribute.

Here is a patch to define the target overridable get_optimization_flag_value
function as well as a set_optimization_flag_value function.  I also
modified opth-gen.awk and optc-gen.awk to make the flags static so that
the rest of the compiler would have to use these functions.  Obviously
the compiler won't build after applying this patch because we would have
to change all the flag references but I thought I would send this
proposal out before doing any more work in that direction.

What do people think?  Does this seem like a reasonable direction to go?
Does anyone see any blockers to this idea?  If we wanted to proceed down
this path it seems like we could either do it with a  big bang approach
(making flags static and change all accesses) or gradually by not making
the flags static and gradually introducing the use of the
get_optimization_flag_value function over time.


2009-05-29  Steve Ellcey  <sje@cup.hp.com>

	PR middle-end/37565
	PR target/37106
	* opth-gen.awk: Remove extern decls of flags.
	Add decls for default_get_optimization_flag_value
	and set_optimization_flag_value.
	* optc-gen.awk: Make flags static.
	Define default_get_optimization_flag_value
	and set_optimization_flag_value.
	* target.h (gcc_target): Add get_optimization_flag_value.
	* target-def.h (TARGET_GET_OPTIMIZATION_FLAG_VALUE): New.


Index: optc-gen.awk
===================================================================
--- optc-gen.awk	(revision 147259)
+++ optc-gen.awk	(working copy)
@@ -534,6 +534,29 @@ print "  if (targetm.target_option.print
 print "    targetm.target_option.print (file, indent, ptr);";
 
 print "}";
+print "";
+print "int";
+print "default_get_flag_value(enum opt_code code)";
+print "{";
+print "#ifdef ENABLE_CHECKING";
+for (i = 0; i < n_opts; i++) {
+	enum = "OPT_" opts[i];
+	if (opts[i] == "finline-limit=" || opts[i] == "Wlarger-than=") {
+		enum = enum "eq";
+	}
+	gsub ("[^A-Za-z0-9]", "_", enum);
+	name = var_name(flags[i]);
+        if (name == "") {
+		print "  if (code == " enum ") gcc_unreachable ();";
+	}
+	else {
+		print "  if (code == " enum " && cl_options[(int) code].flag_var != &" name ") gcc_unreachable ();";
+	}
+}
+print "#endif";
+print "  gcc_assert(cl_options[(int) code].flag_var);";
+print "  return *(cl_options[(int) code].flag_var);";
+print "}";
 print "#endif";
 
 }

Re: RFC: Option handling and PR 37565

[Ian Lance Taylor]

Steve Ellcey <sje@cup.hp.com> writes:

> So instead of 
> 	if (flag_var_tracking)
> we would have
> 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))

I don't particularly want to have to make an indirect function call
every time we check a flag.  I don't see why we should check every time
when we can determine the value once when the compiler starts.

Also, we would still need a place to give backend warnings like

unwind tables currently require either a frame pointer or -maccumulate-outgoing-args for correctness

So I guess I don't much like the idea.

Ian

Re: RFC: Option handling and PR 37565

[Joseph S. Myers]

On Fri, 29 May 2009, Steve Ellcey wrote:

> While looking into PR 37565 I began to wonder if we need to modify how
> we handle optimization flag handling.  Currently we have
> OVERRIDE_OPTIONS, C_COMMON_OVERRIDE_OPTIONS, and OPTIMIZATION_OPTIONS to
> set or override the optimization flags a user gives.  One proposal to
> fix 37565 was to split OVERRIDE_OPTIONS into OVERRIDE_OPTIONS_ALWAYS and
> OVERRIDE_OPTIONS_ONCE  which would create two new macros.
> 
> But I was wondering if a cleaner method to deal with these options would
> be to get rid of all these macros and use target functions to access
> flag values.

I don't really feel that either the original proposal or this one is any 
cleaner than what we have right now, and the original proposal is at least 
simpler.  As neither really addresses the general issues with how options 
are handled (as I see them), on the whole I'd prefer the simpler way of 
fixing the bug that is at least easier to tear out later.

The way I see it, the basic problem is the large amount of ad hoc code, 
much of it target-specific, handling options, their defaults and their 
interactions.  The PR you mention is one problem arising from this; it's 
difficult for options in optimize attributes to work like those on the 
command line.  Another group of problems relate to specs and multilib 
selection; these have no access to the code in the compiler proper 
handling option interactions and defaults and instead make piecemeal 
attempts to replicate bits of that logic, badly.

Both proposals involve new target hooks or macros, moving ad hoc code 
around rather than eliminating it.  These can use arbitrary C code to 
manipulate option state.  This is very flexible, but flexibility can be 
bad; we want consistent rules in GCC for how partially overlapping or 
overriding options should interact, for example, and it should be hard for 
particular targets or options to break those consistent rules 
accidentally.  Consistency tends to argue for making .opt files more 
expressive rather than proliferating hooks or macros.

My general idea is that there should be some sort of object for the 
compiler global state - both target-dependent and and target-independent - 
derived from the command line.  (The cl_optimization structure is a useful 
start to this.)  The compiler should maintain several such objects at 
once.  There would be one for the command line immediately after being 
parsed, before any defaults get applied that would be affected by optimize 
attributes, and a global one with defaults applied that is used by 
references to flag variables.  optimize attributes might create a 
function-local options structure by copying the pre-defaults one, applying 
the function-local options to it and then calling the hooks to apply 
defaults to that structure.  Multilib selection might compute structures 
for each multilib and match them to the structure for the command line.

Where you suggest:

> My idea is that we would never change the values of the flags that are
> set by the user but instead would have a target overridable function
> that returns the value for a given flag by default but in which the
> return value could be overridden for some flags on some targets.
> 
> So instead of 
> 	if (flag_var_tracking)
> we would have
> 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))

I'd instead suggest

#define flag_var_tracking ((int) global_options.optimization.fvar_tracking)

(automatically generated) or similar.  The cast makes it not an lvalue, 
which is a good idea because I also don't want random code changing the 
options; changes to the structure should be limited to particular places 
(automatically generated from .opt files where possible) receiving a 
pointer to the particular structure they are to change.

I think any solution other than a temporary, minimally intrusive fix for 
specific bugs that have been found needs to start with a detailed analysis 
of how and why the present hooks are used in existing targets, resulting 
in an understanding of the requirements for a solution and proposals for 
consistent rules on how options interact.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: RFC: Option handling and PR 37565

[Dave Korn]

Ian Lance Taylor wrote:
> Steve Ellcey <sje@cup.hp.com> writes:
> 
>> So instead of 
>> 	if (flag_var_tracking)
>> we would have
>> 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))
> 
> I don't particularly want to have to make an indirect function call
> every time we check a flag.  I don't see why we should check every time
> when we can determine the value once when the compiler starts.

  The PR is about the interaction between backend optimisation overrides and
"pragma GCC optimize" and the __optimize__ attribute.  Since these change as
compilation proceeds, the backend's choices at compiler startup may need to be
revisited.

  However I also feel it's a bit of a top-heavy solution.  What's wrong with
the simple ONCE/ALWAYS split proposed by HJ in comment #8?

    cheers,
      DaveK

-- 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37565#c8

Re: RFC: Option handling and PR 37565

[Steve Ellcey]

On Fri, 2009-05-29 at 09:38 -0700, Ian Lance Taylor wrote:
> Steve Ellcey <sje@cup.hp.com> writes:
> 
> > So instead of 
> > 	if (flag_var_tracking)
> > we would have
> > 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))
> 
> I don't particularly want to have to make an indirect function call
> every time we check a flag.  I don't see why we should check every time
> when we can determine the value once when the compiler starts.

But if we want to fully support things like the __optimize__ attribute
then we can't just check once when the compiler starts.  The flag could
be changed during compilation by the optimize attribute so at the very
least we need to override some values every time we see this attribute
(which is what we are not doing now).

> Also, we would still need a place to give backend warnings like
> 
> unwind tables currently require either a frame pointer or -maccumulate-outgoing-args for correctness

Warnings are an issue, but again we have the problem that we can't just
do warnings once at the beginning since values could be changed during
compilation.  Maybe the set function should also be overridable so that
it could issue warnings when a variable is set though that wouldn't help
with the above warning since it looks like it needs to be given when
something isn't set.  I am not sure I have an answer to this problem.

> So I guess I don't much like the idea.
> 
> Ian

Steve Ellcey
sje@cup.hp.com

Re: RFC: Option handling and PR 37565

[Steve Ellcey]

On Fri, 2009-05-29 at 16:56 +0000, Joseph S. Myers wrote:

> I don't really feel that either the original proposal or this one is any 
> cleaner than what we have right now, and the original proposal is at least 
> simpler.  As neither really addresses the general issues with how options 
> are handled (as I see them), on the whole I'd prefer the simpler way of 
> fixing the bug that is at least easier to tear out later.

I thought my proposal was cleaner because we could have a rule that says
flags are only changed by user specified flags and by the optimize
attribute.  target code could look at those values and other variables
to determine if it wanted to do a particular optimization or not but it
wouldn't ever change the flag value that was set on the command line or
by the attribute.

> Both proposals involve new target hooks or macros, moving ad hoc code 
> around rather than eliminating it.  These can use arbitrary C code to 
> manipulate option state.  This is very flexible, but flexibility can be 
> bad; we want consistent rules in GCC for how partially overlapping or 
> overriding options should interact, for example, and it should be hard for 
> particular targets or options to break those consistent rules 
> accidentally.  Consistency tends to argue for making .opt files more 
> expressive rather than proliferating hooks or macros.

I think that what I want to do is remove the manipulating (changing) of
the option state completely (except by user flags and the optimization
attribute) and make the use/access of that state more flexible then just
checking a single flag.

> Where you suggest:
> 
> > My idea is that we would never change the values of the flags that are
> > set by the user but instead would have a target overridable function
> > that returns the value for a given flag by default but in which the
> > return value could be overridden for some flags on some targets.
> > 
> > So instead of 
> > 	if (flag_var_tracking)
> > we would have
> > 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))
> 
> I'd instead suggest
> 
> #define flag_var_tracking ((int) global_options.optimization.fvar_tracking)
> 
> (automatically generated) or similar.  The cast makes it not an lvalue, 
> which is a good idea because I also don't want random code changing the 
> options; changes to the structure should be limited to particular places 
> (automatically generated from .opt files where possible) receiving a 
> pointer to the particular structure they are to change.

That would fix Ian's complaint about an indirect function call but it
still means that the decision to do or not do var_tracking is kept in a
single variable that might or might not need to be overridden in some
case.  But I guess in your proposal something in the opt file would tell
us if we want to override the default behaviour for var_tracking and
generate the code that would change it's value from the default.

Steve Ellcey
sje@cup.hp.com

Re: RFC: Option handling and PR 37565

[Ian Lance Taylor]

Steve Ellcey <sje@cup.hp.com> writes:

> On Fri, 2009-05-29 at 09:38 -0700, Ian Lance Taylor wrote:
>> Steve Ellcey <sje@cup.hp.com> writes:
>> 
>> > So instead of 
>> > 	if (flag_var_tracking)
>> > we would have
>> > 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))
>> 
>> I don't particularly want to have to make an indirect function call
>> every time we check a flag.  I don't see why we should check every time
>> when we can determine the value once when the compiler starts.
>
> But if we want to fully support things like the __optimize__ attribute
> then we can't just check once when the compiler starts.  The flag could
> be changed during compilation by the optimize attribute so at the very
> least we need to override some values every time we see this attribute
> (which is what we are not doing now).

Fair enough, but 99.9% of compilations will not use the __optimize__
attribute.  So I still don't want to make an indirect function call
every time we check a flag.

Joseph's suggestion of a more disciplined approach to option
interactions may be the way to go.  We can at least handle many simple
cases that way (this backend always enables/disables this option), while
retaining a C function escape clause if there are complex ones.

Ian

Re: RFC: Option handling and PR 37565

[Michael Meissner]

On Fri, May 29, 2009 at 09:02:23AM -0700, Steve Ellcey wrote:
> While looking into PR 37565 I began to wonder if we need to modify how
> we handle optimization flag handling.  Currently we have
> OVERRIDE_OPTIONS, C_COMMON_OVERRIDE_OPTIONS, and OPTIMIZATION_OPTIONS to
> set or override the optimization flags a user gives.  One proposal to
> fix 37565 was to split OVERRIDE_OPTIONS into OVERRIDE_OPTIONS_ALWAYS and
> OVERRIDE_OPTIONS_ONCE  which would create two new macros.
> 
> But I was wondering if a cleaner method to deal with these options would
> be to get rid of all these macros and use target functions to access
> flag values.
> 
> My idea is that we would never change the values of the flags that are
> set by the user but instead would have a target overridable function
> that returns the value for a given flag by default but in which the
> return value could be overridden for some flags on some targets.
> 
> So instead of 
> 	if (flag_var_tracking)
> we would have
> 	if (targetm.get_optimization_flag_value(OPT_fvar_tracking))

As Ian says, we probably don't want to put a function where we check for each
of the flags.  This will likely be a major slowdown for the compiler.

The trouble is as the compiler gets more complex, it is hard to find just where
the right points to change things are, and what do you want to do about more
global options.  In my original thoughts about target specific support, I
didn't include optimization because it was too hard, but quite a few potential
users of it chimed in of the usefulness of marking a function as a cold
function and compile with -Os instead of a hot function compiling with -O3.
Unfortunately, I have been away from the code for about a year, that I don't
remember all of the details of why particular choices were made.

-- 
Michael Meissner, IBM
4 Technology Place Drive, MS 2203A, Westford, MA, 01886, USA
meissner@linux.vnet.ibm.com