| Message ID | 511E6EF5.6040709@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 1e6afaa..a1d47a6 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -8925,6 +8925,8 @@ handle_optimize_attribute (tree *node, tree name, tree args, DECL_FUNCTION_SPECIFIC_OPTIMIZATION (*node) = build_optimization_node (); + save_optabs_if_changed (*node); + /* Restore current options. */ cl_optimization_restore (&global_options, &cur_opts); } 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 (); + } else restore_target_globals (mips16_globals); } diff --git a/gcc/expr.h b/gcc/expr.h index f5063b4..15fcb47 100644 --- a/gcc/expr.h +++ b/gcc/expr.h @@ -718,7 +718,7 @@ extern bool split_comparison (enum rtx_code, enum machine_mode, /* Call this once to initialize the contents of the optabs appropriately for the current target machine. */ extern void init_optabs (void); -extern void init_all_optabs (void); +extern void init_all_optabs (struct target_optabs *); /* Call this to initialize an optab function entry. */ extern rtx init_one_libfunc (const char *); 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); + } + } + } + this_fn_optabs = fn->optabs ? (struct target_optabs *) fn->optabs + : this_target_optabs; + } } } diff --git a/gcc/function.h b/gcc/function.h index 89d71e5..53e28b7 100644 --- a/gcc/function.h +++ b/gcc/function.h @@ -580,6 +580,9 @@ struct GTY(()) function { a string describing the reason for failure. */ const char * GTY((skip)) cannot_be_copied_reason; + /* Optabs for this function. This is of type `struct target_optabs *'. */ + unsigned char *GTY ((atomic)) optabs; + /* Collected bit flags. */ /* Number of units of general registers that need saving in stdarg diff --git a/gcc/genopinit.c b/gcc/genopinit.c index 1bb2f77..fb80717 100644 --- a/gcc/genopinit.c +++ b/gcc/genopinit.c @@ -422,8 +422,8 @@ main (int argc, char **argv) fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name); fprintf (s_file, "};\n\n"); - fprintf (s_file, "void\ninit_all_optabs (void)\n{\n"); - fprintf (s_file, " bool *ena = this_target_optabs->pat_enable;\n"); + fprintf (s_file, "void\ninit_all_optabs (struct target_optabs *optabs)\n{\n"); + fprintf (s_file, " bool *ena = optabs->pat_enable;\n"); for (i = 0; patterns.iterate (i, &p); ++i) fprintf (s_file, " ena[%u] = HAVE_%s;\n", i, p->name); fprintf (s_file, "}\n\n"); @@ -456,7 +456,7 @@ main (int argc, char **argv) "raw_optab_handler (unsigned scode)\n" "{\n" " int i = lookup_handler (scode);\n" - " return (i >= 0 && this_target_optabs->pat_enable[i]\n" + " return (i >= 0 && this_fn_optabs->pat_enable[i]\n" " ? pats[i].icode : CODE_FOR_nothing);\n" "}\n\n"); @@ -468,8 +468,8 @@ main (int argc, char **argv) " int i = lookup_handler (scode);\n" " if (i >= 0)\n" " {\n" - " bool ret = this_target_optabs->pat_enable[i];\n" - " this_target_optabs->pat_enable[i] = set;\n" + " bool ret = this_fn_optabs->pat_enable[i];\n" + " this_fn_optabs->pat_enable[i] = set;\n" " return ret;\n" " }\n" " else\n" diff --git a/gcc/optabs.c b/gcc/optabs.c index c1dacf4..dd621c3 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3. If not see struct target_optabs default_target_optabs; struct target_libfuncs default_target_libfuncs; +struct target_optabs *this_fn_optabs = &default_target_optabs; #if SWITCHABLE_TARGET struct target_optabs *this_target_optabs = &default_target_optabs; struct target_libfuncs *this_target_libfuncs = &default_target_libfuncs; @@ -6150,7 +6151,7 @@ init_optabs (void) libfunc_hash = htab_create_ggc (10, hash_libfunc, eq_libfunc, NULL); /* Fill in the optabs with the insns we support. */ - init_all_optabs (); + init_all_optabs (this_fn_optabs); /* The ffs function operates on `int'. Fall back on it if we do not have a libgcc2 function for that width. */ @@ -6207,6 +6208,41 @@ init_optabs (void) targetm.init_libfuncs (); } +/* Recompute the optabs and save them if they have changed. */ + +void +save_optabs_if_changed (tree fndecl) +{ + /* ?? If this fails, we should temporarily restore the default + target first (set_cfun (NULL) ??), do the rest of this function, + and then restore it. */ + gcc_assert (this_target_optabs == &default_target_optabs); + + struct target_optabs *tmp_optabs = (struct target_optabs *) + ggc_alloc_atomic (sizeof (struct target_optabs)); + tree optnode = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fndecl); + + /* Generate a new set of optabs into tmp_optabs. */ + init_all_optabs (tmp_optabs); + + /* If the optabs changed, record it. */ + if (memcmp (tmp_optabs, this_target_optabs, sizeof (struct target_optabs))) + { + /* ?? 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)); + + TREE_OPTIMIZATION_OPTABS (optnode) = (unsigned char *) tmp_optabs; + } + else + { + TREE_OPTIMIZATION_OPTABS (optnode) = NULL; + ggc_free (tmp_optabs); + } +} + /* A helper function for init_sync_libfuncs. Using the basename BASE, install libfuncs into TAB for BASE_N for 1 <= N <= MAX. */ diff --git a/gcc/optabs.h b/gcc/optabs.h index c08adcf..4de4409 100644 --- a/gcc/optabs.h +++ b/gcc/optabs.h @@ -76,6 +76,7 @@ struct target_optabs { }; extern struct target_optabs default_target_optabs; +extern struct target_optabs *this_fn_optabs; #if SWITCHABLE_TARGET extern struct target_optabs *this_target_optabs; #else diff --git a/gcc/testsuite/gcc.c-torture/compile/pr52555.c b/gcc/testsuite/gcc.c-torture/compile/pr52555.c new file mode 100644 index 0000000..7016834 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr52555.c @@ -0,0 +1,10 @@ +/* { dg-options "-ffast-math" } */ + +float farg; +unsigned val; + +void __attribute__((optimize("O"))) +test() +{ + val = __builtin_ceilf(farg); +} diff --git a/gcc/tree.h b/gcc/tree.h index c3c814c..740d438 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -3586,14 +3586,25 @@ struct GTY(()) tree_optimization_option { /* The optimization options used by the user. */ struct cl_optimization opts; + + /* Target optabs for this set of optimization options. This is of + type `struct target_optabs *'. */ + unsigned char *GTY ((atomic)) target_optabs; }; #define TREE_OPTIMIZATION(NODE) \ (&OPTIMIZATION_NODE_CHECK (NODE)->optimization.opts) +#define TREE_OPTIMIZATION_OPTABS(NODE) \ + (OPTIMIZATION_NODE_CHECK (NODE)->optimization.target_optabs) + /* Return a tree node that encapsulates the current optimization options. */ extern tree build_optimization_node (void); +/* Save a new set of target_optabs in a TREE_OPTIMIZATION node if the + current set of optabs has changed. */ +extern void save_optabs_if_changed (tree); + /* Target options used by a function. */ struct GTY(()) tree_target_option {