Re: PR target/52555: attribute optimize is overriding command line options

[Richard Sandiford <rdsandiford at googlemail dot com>]

Jakub Jelinek <jakub@redhat.com> writes:
> On Mon, Feb 11, 2013 at 06:15:05PM -0600, Aldy Hernandez wrote:
>> How does this look?
>
> Looks good to me.
>
>> Jakub, what's this you mention in the PR about caching
>> __optimize__((3))?  You also mention I shouldn't compare against
>> this_target_optabs, but default_target_optabs.  But what if
>> this_target_optabs has changed?  (See patch).
>
> The reason for that is that this_target_optabs could at that point be
> simply whatever optabs used the last parsed function.
> this_target_optabs changes only either because of optimize attribute
> (not sure if MIPS as the only switchable target? supports that), or
> because of mips_set_mips16_mode.  I think invoke_set_current_function_hook
> invokes the target hook after the code you've changed, so I'd say it should
> work fine even on MIPS.  CCing Richard for that anyway.

The target hook won't do anything for consecutive functions that
have the same mode though.  It expects this_target_optabs (and other
this_target_* stuff) to stay the same.

Rather than:

	  /* Change optabs if needed.  */
	  if (TREE_OPTIMIZATION_OPTABS (opts))
	    this_target_optabs
	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
	  else
	    this_target_optabs = &default_target_optabs;

I think it'd be better to have:

	  /* Change optabs if needed.  */
	  if (TREE_OPTIMIZATION_OPTABS (opts))
	    this_fn_optabs
	      = (struct target_optabs *) TREE_OPTIMIZATION_OPTABS (opts);
	  else
	    this_fn_optabs = this_target_optabs;

with genopinit.c updated to use this_fn_optabs instead of this_target_optabs.

Richard