Re: [PATCH] New optimize(0) versioning fix (PR target/60026, take 2)
On Fri, 7 Feb 2014, Jan Hubicka wrote: > > On Fri, Feb 07, 2014 at 12:50:22AM +0100, Jan Hubicka wrote: > > > Don't we want to check opt_for_fn (node->decl, cp) instead and arrange > > > -fipa-cp > > > to be false when !optimize? > > > > I can easily imagine using > > !opt_for_fn (node->decl, optimize) > > || !opt_for_fn (node->decl, flag_ipa_cp) > > but guaranteeing flag_ipa_cp or flag_ipa_sra is never true for optimize == 0 > > could be harder, what if something is built with -O0 -fipa-cp or > > __attribute__((optimize (0), "fipa-cp"))) or similar? Checking optimize > > value is among other things about the lack of vdef/vuse for !optimize. > > I always tought it would be better to inform users that -O0 -fipa-cp is > broken combination of flags (but it seems our policy to not do that) > or just clear -fipa-cp while processing argument as we do for some > other contradicting combinations. Well, we have the very same issue for other optimization passes and -O0 (and now also -Og). The idea that you can _enable_ random passes is simply not true. But yes, diagnosing that a switch is ignored would be nice ... Richard.
Re: [PATCH] New optimize(0) versioning fix (PR target/60026, take 2)
> On Fri, Feb 07, 2014 at 12:50:22AM +0100, Jan Hubicka wrote:
> > Don't we want to check opt_for_fn (node->decl, cp) instead and arrange
> > -fipa-cp
> > to be false when !optimize?
>
> I can easily imagine using
> !opt_for_fn (node->decl, optimize)
> || !opt_for_fn (node->decl, flag_ipa_cp)
> but guaranteeing flag_ipa_cp or flag_ipa_sra is never true for optimize == 0
> could be harder, what if something is built with -O0 -fipa-cp or
> __attribute__((optimize (0), "fipa-cp"))) or similar? Checking optimize
> value is among other things about the lack of vdef/vuse for !optimize.
I always tought it would be better to inform users that -O0 -fipa-cp is
broken combination of flags (but it seems our policy to not do that)
or just clear -fipa-cp while processing argument as we do for some
other contradicting combinations.
But yes, lets go with those two checks now, to do what gate does:
static bool
cgraph_gate_cp (void)
{
/* FIXME: We should remove the optimize check after we ensure we never run
IPA passes when not optimizing. */
return flag_ipa_cp && optimize;
}
Honza
>
> Jakub
Re: [PATCH] New optimize(0) versioning fix (PR target/60026, take 2)
On Fri, Feb 07, 2014 at 12:50:22AM +0100, Jan Hubicka wrote: > Don't we want to check opt_for_fn (node->decl, cp) instead and arrange > -fipa-cp > to be false when !optimize? I can easily imagine using !opt_for_fn (node->decl, optimize) || !opt_for_fn (node->decl, flag_ipa_cp) but guaranteeing flag_ipa_cp or flag_ipa_sra is never true for optimize == 0 could be harder, what if something is built with -O0 -fipa-cp or __attribute__((optimize (0), "fipa-cp"))) or similar? Checking optimize value is among other things about the lack of vdef/vuse for !optimize. Jakub
Re: [PATCH] New optimize(0) versioning fix (PR target/60026, take 2)
> On Thu, Feb 06, 2014 at 10:45:23AM +0100, Richard Biener wrote:
> > On Thu, 6 Feb 2014, Jakub Jelinek wrote:
> >
> > > On Wed, Feb 05, 2014 at 08:42:27PM +0100, Jakub Jelinek wrote:
> > > > So, where do we want to do that instead? E.g. should it be e.g. in
> > > > tree_versionable_function_p directly and let the inliner (if it doesn't
> > > > do
> > > > already) also treat optimize(0) functions that aren't always_inline as
> > > > noinline?
> > >
> > > So, another attempt to put the && opt_for_fn (fndecl, optimize) into
> > > tree_versionable_function_p also failed, because e.g. for TM (but also for
> > > SIMD clones) we need to clone -O0 functions. So, can I fix PR600{6,7}2
> > > without touching tree-inline.c and fix PR60026 again separately somehow
> > > else
> > > as follow-up? Bootstrapped/regtested on x86_64-linux and i686-linux.
> >
> > That works for me.
>
> Here it is. Bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk?
>
> 2014-02-06 Jakub Jelinek
>
> PR ipa/60026
> * ipa-cp.c (determine_versionability): Fail at -O0
> or __attribute__((optimize (0))) functions.
> * tree-sra.c (ipa_sra_preliminary_function_checks): Likewise.
>
> Revert:
> 2014-02-04 Jakub Jelinek
>
> PR ipa/60026
> * tree-inline.c (copy_forbidden): Fail for
> __attribute__((optimize (0))) functions.
>
> --- gcc/tree-inline.c.jj 2014-02-05 16:18:50.0 +0100
> +++ gcc/tree-inline.c 2014-02-06 17:12:03.008993548 +0100
> @@ -3315,18 +3315,6 @@ copy_forbidden (struct function *fun, tr
> goto fail;
>}
>
> - tree fs_opts;
> - fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fun->decl);
> - if (fs_opts)
> -{
> - struct cl_optimization *os = TREE_OPTIMIZATION (fs_opts);
> - if (!os->x_optimize)
> - {
> - reason = G_("function %q+F compiled without optimizations");
> - goto fail;
> - }
> -}
> -
> fail:
>fun->cannot_be_copied_reason = reason;
>fun->cannot_be_copied_set = true;
> --- gcc/tree-sra.c.jj 2014-01-03 11:40:57.0 +0100
> +++ gcc/tree-sra.c2014-02-06 17:15:43.348804011 +0100
> @@ -4900,6 +4900,13 @@ ipa_sra_preliminary_function_checks (str
>return false;
> }
>
> + if (!opt_for_fn (node->decl, optimize))
> +{
> + if (dump_file)
> + fprintf (dump_file, "Function not optimized.\n");
> + return false;
> +}
Don't we want to check opt_for_fn (node->decl, cp) instead and arrange -fipa-cp
to be false when !optimize?
Otherwise it is OK. I would like to move IPA passes to check the flags
on function basis instead of withing global gates - that seems only way
I can imagine IPA passes to work at LTO with presnece of multiple command
line options in longer run.
Thanks,
Honza
> +
>if (DECL_VIRTUAL_P (current_function_decl))
> {
>if (dump_file)
> --- gcc/ipa-cp.c.jj 2014-02-05 10:38:01.0 +0100
> +++ gcc/ipa-cp.c 2014-02-06 17:19:13.405671970 +0100
> @@ -430,6 +430,8 @@ determine_versionability (struct cgraph_
> reason = "not a tree_versionable_function";
>else if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
> reason = "insufficient body availability";
> + else if (!opt_for_fn (node->decl, optimize))
> +reason = "non-optimized function";
>else if (lookup_attribute ("omp declare simd", DECL_ATTRIBUTES
> (node->decl)))
> {
>/* Ideally we should clone the SIMD clones themselves and create
>
>
> Jakub
[PATCH] New optimize(0) versioning fix (PR target/60026, take 2)
On Thu, Feb 06, 2014 at 10:45:23AM +0100, Richard Biener wrote:
> On Thu, 6 Feb 2014, Jakub Jelinek wrote:
>
> > On Wed, Feb 05, 2014 at 08:42:27PM +0100, Jakub Jelinek wrote:
> > > So, where do we want to do that instead? E.g. should it be e.g. in
> > > tree_versionable_function_p directly and let the inliner (if it doesn't do
> > > already) also treat optimize(0) functions that aren't always_inline as
> > > noinline?
> >
> > So, another attempt to put the && opt_for_fn (fndecl, optimize) into
> > tree_versionable_function_p also failed, because e.g. for TM (but also for
> > SIMD clones) we need to clone -O0 functions. So, can I fix PR600{6,7}2
> > without touching tree-inline.c and fix PR60026 again separately somehow else
> > as follow-up? Bootstrapped/regtested on x86_64-linux and i686-linux.
>
> That works for me.
Here it is. Bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?
2014-02-06 Jakub Jelinek
PR ipa/60026
* ipa-cp.c (determine_versionability): Fail at -O0
or __attribute__((optimize (0))) functions.
* tree-sra.c (ipa_sra_preliminary_function_checks): Likewise.
Revert:
2014-02-04 Jakub Jelinek
PR ipa/60026
* tree-inline.c (copy_forbidden): Fail for
__attribute__((optimize (0))) functions.
--- gcc/tree-inline.c.jj2014-02-05 16:18:50.0 +0100
+++ gcc/tree-inline.c 2014-02-06 17:12:03.008993548 +0100
@@ -3315,18 +3315,6 @@ copy_forbidden (struct function *fun, tr
goto fail;
}
- tree fs_opts;
- fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fun->decl);
- if (fs_opts)
-{
- struct cl_optimization *os = TREE_OPTIMIZATION (fs_opts);
- if (!os->x_optimize)
- {
- reason = G_("function %q+F compiled without optimizations");
- goto fail;
- }
-}
-
fail:
fun->cannot_be_copied_reason = reason;
fun->cannot_be_copied_set = true;
--- gcc/tree-sra.c.jj 2014-01-03 11:40:57.0 +0100
+++ gcc/tree-sra.c 2014-02-06 17:15:43.348804011 +0100
@@ -4900,6 +4900,13 @@ ipa_sra_preliminary_function_checks (str
return false;
}
+ if (!opt_for_fn (node->decl, optimize))
+{
+ if (dump_file)
+ fprintf (dump_file, "Function not optimized.\n");
+ return false;
+}
+
if (DECL_VIRTUAL_P (current_function_decl))
{
if (dump_file)
--- gcc/ipa-cp.c.jj 2014-02-05 10:38:01.0 +0100
+++ gcc/ipa-cp.c2014-02-06 17:19:13.405671970 +0100
@@ -430,6 +430,8 @@ determine_versionability (struct cgraph_
reason = "not a tree_versionable_function";
else if (cgraph_function_body_availability (node) <= AVAIL_OVERWRITABLE)
reason = "insufficient body availability";
+ else if (!opt_for_fn (node->decl, optimize))
+reason = "non-optimized function";
else if (lookup_attribute ("omp declare simd", DECL_ATTRIBUTES (node->decl)))
{
/* Ideally we should clone the SIMD clones themselves and create
Jakub
