* Question about inlining and strict_aliasing
@ 2017-01-09 12:40 Martin Liška
2017-01-09 13:34 ` Jan Hubicka
0 siblings, 1 reply; 3+ messages in thread
From: Martin Liška @ 2017-01-09 12:40 UTC (permalink / raw)
To: GCC Development; +Cc: Richard Biener, Jan Hubicka
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
Hello.
I've been working on a patch that would cope with target and optimization (read PerFunction)
in a proper way. I came to following test-case (slightly modified ./gcc/testsuite/gcc.c-torture/execute/alias-1.c):
int val;
int *ptr = &val;
float *ptr2 = &val;
static
__attribute__((always_inline, optimize ("-fno-strict-aliasing")))
typepun ()
{
*ptr2=0;
}
main()
{
*ptr=1;
typepun ();
if (*ptr)
__builtin_abort ();
}
$ gcc -O2 /tmp/always-inline.c -fdump-tree-all-details && ./a.out
Aborted (core dumped)
Problem is that einline does:
Inlining typepun into main (always_inline).
Estimating body: typepun/3
Known to be false: not inlined
size:2 time:2
Dropping flag_strict_aliasing on main:4 <<---- here is strict_aliasing drop
Accounting size:2.00, time:2.00 on predicate:(true)
However fre1 does not still check for flag_strict_aliasing. Is it bug or not?
I did an experimental fix, but I guess there will me multiple places where the
flag is checked.
Second question:
Current ipa-inline.c code contains question sophisticated check_{match,maybe_up,maybe_down} and
it's following by:
/* When user added an attribute to the callee honor it. */
else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl))
&& opts_for_fn (caller->decl) != opts_for_fn (callee->decl))
{
e->inline_failed = CIF_OPTIMIZATION_MISMATCH;
inlinable = false;
}
I think it's very strict, as when one uses __attribute__((optimize)) with a param that matches with
command line arguments, it will return false. What if we remove the hunk?
Thanks,
Martin
[-- Attachment #2: einline.patch --]
[-- Type: text/x-patch, Size: 1599 bytes --]
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 871fa121fd0..da38e4fab69 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -986,7 +986,7 @@ ncr_compar (const void *field1_, const void *field2_)
static bool
nonoverlapping_component_refs_p (const_tree x, const_tree y)
{
- if (!flag_strict_aliasing
+ if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing
|| !x || !y
|| TREE_CODE (x) != COMPONENT_REF
|| TREE_CODE (y) != COMPONENT_REF)
@@ -1167,7 +1167,7 @@ indirect_ref_may_alias_decl_p (tree ref1 ATTRIBUTE_UNUSED, tree base1,
return false;
/* Disambiguations that rely on strict aliasing rules follow. */
- if (!flag_strict_aliasing || !tbaa_p)
+ if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p)
return true;
ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
@@ -1334,7 +1334,7 @@ indirect_refs_may_alias_p (tree ref1 ATTRIBUTE_UNUSED, tree base1,
return false;
/* Disambiguations that rely on strict aliasing rules follow. */
- if (!flag_strict_aliasing || !tbaa_p)
+ if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p)
return true;
ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1));
@@ -1508,7 +1508,7 @@ refs_may_alias_p_1 (ao_ref *ref1, ao_ref *ref2, bool tbaa_p)
/* First defer to TBAA if possible. */
if (tbaa_p
- && flag_strict_aliasing
+ && opts_for_fn (current_function_decl)->x_flag_strict_aliasing
&& !alias_sets_conflict_p (ao_ref_alias_set (ref1),
ao_ref_alias_set (ref2)))
return false;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: Question about inlining and strict_aliasing 2017-01-09 12:40 Question about inlining and strict_aliasing Martin Liška @ 2017-01-09 13:34 ` Jan Hubicka 2017-01-09 14:08 ` Martin Liška 0 siblings, 1 reply; 3+ messages in thread From: Jan Hubicka @ 2017-01-09 13:34 UTC (permalink / raw) To: Martin Liška; +Cc: GCC Development, Richard Biener, Jan Hubicka > Hello. > > I've been working on a patch that would cope with target and optimization (read PerFunction) > in a proper way. I came to following test-case (slightly modified ./gcc/testsuite/gcc.c-torture/execute/alias-1.c): > > int val; > > int *ptr = &val; > float *ptr2 = &val; > > static > __attribute__((always_inline, optimize ("-fno-strict-aliasing"))) > typepun () > { > *ptr2=0; > } > > main() > { > *ptr=1; > typepun (); > if (*ptr) > __builtin_abort (); > } > > $ gcc -O2 /tmp/always-inline.c -fdump-tree-all-details && ./a.out > Aborted (core dumped) > > Problem is that einline does: > > Inlining typepun into main (always_inline). > Estimating body: typepun/3 > Known to be false: not inlined > size:2 time:2 > Dropping flag_strict_aliasing on main:4 <<---- here is strict_aliasing drop > Accounting size:2.00, time:2.00 on predicate:(true) > > However fre1 does not still check for flag_strict_aliasing. Is it bug or not? > I did an experimental fix, but I guess there will me multiple places where the > flag is checked. Aha, I think the problem is that once we modify strict aliasing of cfun, we need to re-set the global optimization attributes, becuase those are still copies from the original declaration. > > Second question: > Current ipa-inline.c code contains question sophisticated check_{match,maybe_up,maybe_down} and > it's following by: > > /* When user added an attribute to the callee honor it. */ > else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl)) > && opts_for_fn (caller->decl) != opts_for_fn (callee->decl)) > { > e->inline_failed = CIF_OPTIMIZATION_MISMATCH; > inlinable = false; > } > > I think it's very strict, as when one uses __attribute__((optimize)) with a param that matches with > command line arguments, it will return false. What if we remove the hunk? I am not quite sure. If you have function A with optimization attribute and you inline it to function b with same command line parameters, it may enable to inline it into function C where A would not be inlined because of the explicit optimization attribute. If we relax this check, we will need to copy the optimization attribute from A to B at the time we inline into B. > > Thanks, > Martin > diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c > index 871fa121fd0..da38e4fab69 100644 > --- a/gcc/tree-ssa-alias.c > +++ b/gcc/tree-ssa-alias.c > @@ -986,7 +986,7 @@ ncr_compar (const void *field1_, const void *field2_) > static bool > nonoverlapping_component_refs_p (const_tree x, const_tree y) > { > - if (!flag_strict_aliasing > + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing This should not be necessary. Einline ought to update global opts. I suppose when decl modified is cfun one needs to reset current_funcion_decl to NULL and then set it again so the global flags changing machinery notices this change. Honza t > || !x || !y > || TREE_CODE (x) != COMPONENT_REF > || TREE_CODE (y) != COMPONENT_REF) > @@ -1167,7 +1167,7 @@ indirect_ref_may_alias_decl_p (tree ref1 ATTRIBUTE_UNUSED, tree base1, > return false; > > /* Disambiguations that rely on strict aliasing rules follow. */ > - if (!flag_strict_aliasing || !tbaa_p) > + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p) > return true; > > ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1)); > @@ -1334,7 +1334,7 @@ indirect_refs_may_alias_p (tree ref1 ATTRIBUTE_UNUSED, tree base1, > return false; > > /* Disambiguations that rely on strict aliasing rules follow. */ > - if (!flag_strict_aliasing || !tbaa_p) > + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p) > return true; > > ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1)); > @@ -1508,7 +1508,7 @@ refs_may_alias_p_1 (ao_ref *ref1, ao_ref *ref2, bool tbaa_p) > > /* First defer to TBAA if possible. */ > if (tbaa_p > - && flag_strict_aliasing > + && opts_for_fn (current_function_decl)->x_flag_strict_aliasing > && !alias_sets_conflict_p (ao_ref_alias_set (ref1), > ao_ref_alias_set (ref2))) > return false; ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Question about inlining and strict_aliasing 2017-01-09 13:34 ` Jan Hubicka @ 2017-01-09 14:08 ` Martin Liška 0 siblings, 0 replies; 3+ messages in thread From: Martin Liška @ 2017-01-09 14:08 UTC (permalink / raw) To: Jan Hubicka; +Cc: GCC Development, Richard Biener [-- Attachment #1: Type: text/plain, Size: 4799 bytes --] On 01/09/2017 02:34 PM, Jan Hubicka wrote: >> Hello. >> >> I've been working on a patch that would cope with target and optimization (read PerFunction) >> in a proper way. I came to following test-case (slightly modified ./gcc/testsuite/gcc.c-torture/execute/alias-1.c): >> >> int val; >> >> int *ptr = &val; >> float *ptr2 = &val; >> >> static >> __attribute__((always_inline, optimize ("-fno-strict-aliasing"))) >> typepun () >> { >> *ptr2=0; >> } >> >> main() >> { >> *ptr=1; >> typepun (); >> if (*ptr) >> __builtin_abort (); >> } >> >> $ gcc -O2 /tmp/always-inline.c -fdump-tree-all-details && ./a.out >> Aborted (core dumped) >> >> Problem is that einline does: >> >> Inlining typepun into main (always_inline). >> Estimating body: typepun/3 >> Known to be false: not inlined >> size:2 time:2 >> Dropping flag_strict_aliasing on main:4 <<---- here is strict_aliasing drop >> Accounting size:2.00, time:2.00 on predicate:(true) >> >> However fre1 does not still check for flag_strict_aliasing. Is it bug or not? >> I did an experimental fix, but I guess there will me multiple places where the >> flag is checked. > > Aha, I think the problem is that once we modify strict aliasing of cfun, we need > to re-set the global optimization attributes, becuase those are still copies from > the original declaration. Good, I prepared patch that does that and works for me. Is it stage3 material? I hope it is as it fixes a miscompilation. Should I create PR for that? >> >> Second question: >> Current ipa-inline.c code contains question sophisticated check_{match,maybe_up,maybe_down} and >> it's following by: >> >> /* When user added an attribute to the callee honor it. */ >> else if (lookup_attribute ("optimize", DECL_ATTRIBUTES (callee->decl)) >> && opts_for_fn (caller->decl) != opts_for_fn (callee->decl)) >> { >> e->inline_failed = CIF_OPTIMIZATION_MISMATCH; >> inlinable = false; >> } >> >> I think it's very strict, as when one uses __attribute__((optimize)) with a param that matches with >> command line arguments, it will return false. What if we remove the hunk? > > I am not quite sure. If you have function A with optimization attribute and you inline it to function b > with same command line parameters, it may enable to inline it into function C where A would not be inlined > because of the explicit optimization attribute. > If we relax this check, we will need to copy the optimization attribute from A to B at the time > we inline into B. Can you please provide an example for that? As I've been thinking about it, both IPA inline and IPA ICF should handle optimization flags/attributes in very similar manner as it can potentially make a divergence for -fcompare-debug >> >> Thanks, >> Martin > >> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c >> index 871fa121fd0..da38e4fab69 100644 >> --- a/gcc/tree-ssa-alias.c >> +++ b/gcc/tree-ssa-alias.c >> @@ -986,7 +986,7 @@ ncr_compar (const void *field1_, const void *field2_) >> static bool >> nonoverlapping_component_refs_p (const_tree x, const_tree y) >> { >> - if (!flag_strict_aliasing >> + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing > > This should not be necessary. Einline ought to update global opts. > I suppose when decl modified is cfun one needs to reset current_funcion_decl > to NULL and then set it again so the global flags changing machinery notices this > change. Yep, fixed in the patch Martin > > Honza > t >> || !x || !y >> || TREE_CODE (x) != COMPONENT_REF >> || TREE_CODE (y) != COMPONENT_REF) >> @@ -1167,7 +1167,7 @@ indirect_ref_may_alias_decl_p (tree ref1 ATTRIBUTE_UNUSED, tree base1, >> return false; >> >> /* Disambiguations that rely on strict aliasing rules follow. */ >> - if (!flag_strict_aliasing || !tbaa_p) >> + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p) >> return true; >> >> ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1)); >> @@ -1334,7 +1334,7 @@ indirect_refs_may_alias_p (tree ref1 ATTRIBUTE_UNUSED, tree base1, >> return false; >> >> /* Disambiguations that rely on strict aliasing rules follow. */ >> - if (!flag_strict_aliasing || !tbaa_p) >> + if (!opts_for_fn (current_function_decl)->x_flag_strict_aliasing || !tbaa_p) >> return true; >> >> ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1)); >> @@ -1508,7 +1508,7 @@ refs_may_alias_p_1 (ao_ref *ref1, ao_ref *ref2, bool tbaa_p) >> >> /* First defer to TBAA if possible. */ >> if (tbaa_p >> - && flag_strict_aliasing >> + && opts_for_fn (current_function_decl)->x_flag_strict_aliasing >> && !alias_sets_conflict_p (ao_ref_alias_set (ref1), >> ao_ref_alias_set (ref2))) >> return false; > [-- Attachment #2: einline-v2.patch --] [-- Type: text/x-patch, Size: 2473 bytes --] diff --git a/gcc/function.c b/gcc/function.c index 7fde96adbe3..f625489205b 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4811,9 +4811,9 @@ invoke_set_current_function_hook (tree fndecl) /* cfun should never be set directly; use this function. */ void -set_cfun (struct function *new_cfun) +set_cfun (struct function *new_cfun, bool force) { - if (cfun != new_cfun) + if (cfun != new_cfun || force) { cfun = new_cfun; invoke_set_current_function_hook (new_cfun ? new_cfun->decl : NULL_TREE); diff --git a/gcc/function.h b/gcc/function.h index fb3cbbc3346..fb8f57ae385 100644 --- a/gcc/function.h +++ b/gcc/function.h @@ -613,7 +613,7 @@ extern tree block_chainon (tree, tree); extern void number_blocks (tree); /* cfun shouldn't be set directly; use one of these functions instead. */ -extern void set_cfun (struct function *new_cfun); +extern void set_cfun (struct function *new_cfun, bool force = false); extern void push_cfun (struct function *new_cfun); extern void pop_cfun (void); diff --git a/gcc/ipa-inline-transform.c b/gcc/ipa-inline-transform.c index 7725fadd6d8..a8e73cd6967 100644 --- a/gcc/ipa-inline-transform.c +++ b/gcc/ipa-inline-transform.c @@ -340,6 +340,8 @@ inline_call (struct cgraph_edge *e, bool update_original, if (DECL_FUNCTION_PERSONALITY (callee->decl)) DECL_FUNCTION_PERSONALITY (to->decl) = DECL_FUNCTION_PERSONALITY (callee->decl); + + bool reload_optimization_node = false; if (!opt_for_fn (callee->decl, flag_strict_aliasing) && opt_for_fn (to->decl, flag_strict_aliasing)) { @@ -352,6 +354,7 @@ inline_call (struct cgraph_edge *e, bool update_original, to->name (), to->order); DECL_FUNCTION_SPECIFIC_OPTIMIZATION (to->decl) = build_optimization_node (&opts); + reload_optimization_node = true; } inline_summary *caller_info = inline_summaries->get (to); @@ -412,9 +415,14 @@ inline_call (struct cgraph_edge *e, bool update_original, callee->name (), callee->order, to->name (), to->order); DECL_FUNCTION_SPECIFIC_OPTIMIZATION (to->decl) = build_optimization_node (&opts); + reload_optimization_node = true; } } + /* Reload global optimization flags. */ + if (reload_optimization_node && DECL_STRUCT_FUNCTION (to->decl) == cfun) + set_cfun (cfun, true); + /* If aliases are involved, redirect edge to the actual destination and possibly remove the aliases. */ if (e->callee != callee) ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-09 14:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-09 12:40 Question about inlining and strict_aliasing Martin Liška 2017-01-09 13:34 ` Jan Hubicka 2017-01-09 14:08 ` Martin Liška
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).