This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Aldy Hernandez <aldyh@redhat.com> writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index b203cdd..5e98485 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p)
>    if (mips16_p)
>      {
>        if (!mips16_globals)
> -	mips16_globals = save_target_globals ();
> +	{
> +	  if (optimization_current_node != optimization_default_node)
> +	    {
> +	      tree opts = optimization_current_node;
> +	      /* Temporarily switch to the default optimization node,
> +		 so that *this_target_optabs is set to the default,
> +		 not reflecting whatever the first mips16 function
> +		 uses for the optimize attribute.  */
> +	      optimization_current_node = optimization_default_node;
> +	      cl_optimization_restore
> +		(&global_options,
> +		 TREE_OPTIMIZATION (optimization_default_node));
> +	      mips16_globals = save_target_globals ();
> +	      optimization_current_node = opts;
> +	      cl_optimization_restore (&global_options,
> +				       TREE_OPTIMIZATION (opts));
> +	    }
> +	  else
> +	    mips16_globals = save_target_globals ();	  
> +	}

I think this should go in target-globals.c, maybe as
save_target_globals_default_opts.

> diff --git a/gcc/function.c b/gcc/function.c
> index 4ce2259..c5eea2e 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -4400,6 +4400,31 @@ invoke_set_current_function_hook (tree fndecl)
>  	}
>  
>        targetm.set_current_function (fndecl);
> +
> +      if (opts == optimization_default_node)
> +	this_fn_optabs = this_target_optabs;
> +      else
> +	{
> +	  struct function *fn = DECL_STRUCT_FUNCTION (fndecl);
> +	  if (fn->optabs == NULL)
> +	    {
> +	      if (!SWITCHABLE_TARGET)
> +		fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);
> +	      else
> +		{
> +		  if (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);
> +		    }
> +		}

Following on from Jakub's:

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

I'd prefer just:

		if (this_target_optabs == &default_target_optabs)
		  fn->optabs = TREE_OPTIMIZATION_OPTABS (opts);

Looks good to me otherwise, thanks.

Sorry that SWITCHABLE_TARGETS has been so much hassle.  TBH, like Jakub
says in the PR, I was hoping things like the optimize attribute could
use the target_globals stuff too.  Today we just care about optabs,
but e.g. arm.c has:

  if (TARGET_THUMB1 && optimize_size)
    {
      /* When optimizing for size on Thumb-1, it's better not
        to use the HI regs, because of the overhead of
        stacking them.  */
      for (regno = FIRST_HI_REGNUM;
	   regno <= LAST_HI_REGNUM; ++regno)
	fixed_regs[regno] = call_used_regs[regno] = 1;
    }

which affects other cached global state like IRA tables.

The rtx_costs also often depend on optimize_size, and are cached in
various places like expmed.c.  E.g. for:

int
foo (int x, int y)
{
  return x * 10;
}

the -O2 version on MIPS32 is:

        sll     $2,$4,1
        sll     $4,$4,3
        j       $31
        addu    $2,$2,$4

and the -Os version is:

        li      $2,10
        j       $31
        mul     $2,$4,$2

But even if an optimize attribute is added:

int __attribute__((optimize(2)))
foo (int x, int y)
{
  return x * 10;
}

what you get depends on whether -Os or -O2 was passed on the command line.
The attribute doesn't affect things either way.  The same thing happens
on x86_64.

So in the end I think we'll end up trying solve the same problem that
the SWITCHABLE_TARGETS stuff was trying to solve, but this time for
__attribute__((optimize)).

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]