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

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Feb 15, 2013 at 11:23:01AM -0600, Aldy Hernandez wrote:
> +2013-02-15  Aldy Hernandez  <aldyh@redhat.com>
> +	    Jakub Jelinek  <jakub@redhat.com>
> +
> +	PR target/52555
> +	* genopinit.c (raw_optab_handler): Use this_fn_optabs.
> +	(swap_optab_enable): Same.
> +	(init_all_optabs): Use argument instead of global.
> +	* tree.h (struct tree_optimization_option): New field
> +	target_optabs.
> +	* expr.h (init_all_optabs): Add argument to prototype.
> +	(TREE_OPTIMIZATION_OPTABS): New.
> +	(save_optabs_if_changed): Protoize.
> +	* optabs.h: Declare this_fn_optabs.
> +	* optabs.c (save_optabs_if_changed): New.
> +	Declare this_fn_optabs.
> +	(init_optabs): Add argument to init_all_optabs() call.
> +	* function.c (invoke_set_current_function_hook): Handle per
> +	function optabs.
> +	* function.h (struct function): New field optabs.
> +	* config/mips/mips.c (mips_set_mips16_mode): Handle when
> +	optimization_current_node has changed.
> c/family
> +	PR target/52555
> +	* c-common.c (handle_optimize_attribute): Call
> +	save_optabs_if_changed.

Looks good, just a few nits.  But please wait for Richard's feedback on it.

> +	      if (!SWITCHABLE_TARGET)
> +		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +	      else
> +		{
> +		  if (this_target_optabs == &default_target_optabs)
> +		    fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

Just use
		if (!SWITCHABLE_TARGET
		    || this_target_optabs == &default_target_optabs)
		  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

> +		  else
> +		    {
> +		      fn->optabs = (unsigned char *)
> +			ggc_alloc_atomic (sizeof (struct target_optabs));
> +		      init_all_optabs ((struct target_optabs *) fn->optabs);
> +		    }
> +		}

and reindent.

> +	    }
> +	  this_fn_optabs = fn->optabs ? (struct target_optabs *) fn->optabs
> +	    : this_target_optabs;

I'd prefer : here be below ? on the line above it.

> +      /* ?? An existing optabs indicates multiple ((optimize))
> +	 attributes for the same function.  Is this even valid?  For
> +	 now, just clobber the existing entry with the new optabs.  */
> +      if (TREE_OPTIMIZATION_OPTABS (optnode))
> +	XDELETE (TREE_OPTIMIZATION_OPTABS (optnode));

The comment is wrong,
void foo (void) __attribute__((optimize (2)));
void foo (void) __attribute__((optimize ("fast-math")));
void foo (void) __attribute__((optimize ("unroll-loops")));

void
foo (void)
{
}
is just fine, and for foo results in -O2 -ffast-math -funroll-loops
options being in effect.
Plus XDELETE on GC allocated memory is wrong.  Just remove the comment
and if and XDELETE lines.

	Jakub