* 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).