Re: Question about inlining and strict_aliasing

[Martin Liška <mliska at suse dot cz>]

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;
> 

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)