On Fri, Feb 15, 2013 at 11:23:01AM -0600, Aldy Hernandez wrote:
> +2013-02-15  Aldy Hernandez  <al...@redhat.com>
> +         Jakub Jelinek  <ja...@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

Reply via email to