[PATCH][C++] Remove auto_var_in_fn langhook

[PATCH][C++] Remove auto_var_in_fn langhook

[Richard Guenther]

The langhook is currently only overridden by the C++ frontend but its
implementation doesn't add value over the generic implementation (I 
believe after careful looking).

Full bootstrap and regtest still running but the patch has survived
a build and C++ regtest already.

Ok for mainline?

Thanks,
Richard.

2007-07-26  Richard Guenther  <rguenther@suse.de>

	* langhooks-def.h (lhd_tree_inlining_auto_var_in_fn_p): Remove.
	(LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P): Likewise.
	(LANG_HOOKS_TREE_INLINING_INITIALIZER): Remove initializer for
	auto_var_in_fn_p langhook.
	* langhooks.c (lhd_tree_inlining_auto_var_in_fn_p): Rename and
	move ...
	* tree.c (auto_var_in_fn_p): ... here.
	(find_var_from_fn): Call auto_var_in_fn_p directly.
	* langhooks.h (lang_hooks_for_tree_inlining): Remove
	auto_var_in_fn_p langhook.
	* tree-inline.c (remap_decls): Call auto_var_in_fn_p directly.
	(copy_body_r): Likewise.
	(self_inlining_addr_expr): Likewise.
	* tree.h (auto_var_in_fn_p): Declare.

	cp/
	* cp-objcp-common.h (LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P):
	Remove.
	* cp-tree.h (cp_auto_var_in_fn_p): Remove.
	* tree.c (cp_auto_var_in_fn_p): Remove.

Index: gcc/cp/cp-objcp-common.h
===================================================================
*** gcc.orig/cp/cp-objcp-common.h	2007-07-26 17:19:36.000000000 +0200
--- gcc/cp/cp-objcp-common.h	2007-07-26 17:25:02.000000000 +0200
*************** extern tree objcp_tsubst_copy_and_build 
*** 108,116 ****
  #undef LANG_HOOKS_TREE_INLINING_CANNOT_INLINE_TREE_FN
  #define LANG_HOOKS_TREE_INLINING_CANNOT_INLINE_TREE_FN \
    cp_cannot_inline_tree_fn
- #undef LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P
- #define LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P \
-   cp_auto_var_in_fn_p
  #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P
  #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P cp_var_mod_type_p
  #undef LANG_HOOKS_TREE_DUMP_DUMP_TREE_FN
--- 108,113 ----
Index: gcc/cp/cp-tree.h
===================================================================
*** gcc.orig/cp/cp-tree.h	2007-07-26 17:19:36.000000000 +0200
--- gcc/cp/cp-tree.h	2007-07-26 17:26:32.000000000 +0200
*************** extern tree cp_walk_subtrees (tree*, int
*** 4730,4736 ****
  #define cp_walk_tree_without_duplicates(a,b,c) \
  	walk_tree_without_duplicates_1 (a, b, c, cp_walk_subtrees)
  extern int cp_cannot_inline_tree_fn		(tree*);
- extern int cp_auto_var_in_fn_p			(tree,tree);
  extern tree fold_if_not_in_template		(tree);
  extern tree rvalue				(tree);
  extern tree convert_bitfield_to_declared_type   (tree);
--- 4730,4735 ----
Index: gcc/cp/tree.c
===================================================================
*** gcc.orig/cp/tree.c	2007-07-26 17:19:36.000000000 +0200
--- gcc/cp/tree.c	2007-07-26 17:24:47.000000000 +0200
*************** cp_cannot_inline_tree_fn (tree* fnp)
*** 2443,2458 ****
    return 0;
  }
  
- /* Determine whether VAR is a declaration of an automatic variable in
-    function FN.  */
- 
- int
- cp_auto_var_in_fn_p (tree var, tree fn)
- {
-   return (DECL_P (var) && DECL_CONTEXT (var) == fn
- 	  && nonstatic_local_decl_p (var));
- }
- 
  /* Like save_expr, but for C++.  */
  
  tree
--- 2443,2448 ----
Index: gcc/langhooks-def.h
===================================================================
*** gcc.orig/langhooks-def.h	2007-07-26 17:19:36.000000000 +0200
--- gcc/langhooks-def.h	2007-07-26 17:24:12.000000000 +0200
*************** extern tree lhd_builtin_function (tree d
*** 69,75 ****
  /* Declarations of default tree inlining hooks.  */
  extern int lhd_tree_inlining_cannot_inline_tree_fn (tree *);
  extern int lhd_tree_inlining_disregard_inline_limits (tree);
- extern int lhd_tree_inlining_auto_var_in_fn_p (tree, tree);
  extern void lhd_initialize_diagnostics (struct diagnostic_context *);
  extern tree lhd_callgraph_analyze_expr (tree *, int *, tree);
  
--- 69,74 ----
*************** extern void lhd_omp_firstprivatize_type_
*** 136,150 ****
    lhd_tree_inlining_cannot_inline_tree_fn
  #define LANG_HOOKS_TREE_INLINING_DISREGARD_INLINE_LIMITS \
    lhd_tree_inlining_disregard_inline_limits
- #define LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P \
-   lhd_tree_inlining_auto_var_in_fn_p
  #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P \
    hook_bool_tree_tree_false
  
  #define LANG_HOOKS_TREE_INLINING_INITIALIZER { \
    LANG_HOOKS_TREE_INLINING_CANNOT_INLINE_TREE_FN, \
    LANG_HOOKS_TREE_INLINING_DISREGARD_INLINE_LIMITS, \
-   LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P, \
    LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P, \
  }
  
--- 135,146 ----
Index: gcc/langhooks.c
===================================================================
*** gcc.orig/langhooks.c	2007-07-26 17:19:36.000000000 +0200
--- gcc/langhooks.c	2007-07-26 17:21:34.000000000 +0200
*************** lhd_tree_inlining_disregard_inline_limit
*** 295,313 ****
    return 0;
  }
  
- /* lang_hooks.tree_inlining.auto_var_in_fn_p is called to determine
-    whether VT is an automatic variable defined in function FT.  */
- 
- int
- lhd_tree_inlining_auto_var_in_fn_p (tree var, tree fn)
- {
-   return (DECL_P (var) && DECL_CONTEXT (var) == fn
- 	  && (((TREE_CODE (var) == VAR_DECL || TREE_CODE (var) == PARM_DECL)
- 	       && ! TREE_STATIC (var))
- 	      || TREE_CODE (var) == LABEL_DECL
- 	      || TREE_CODE (var) == RESULT_DECL));
- }
- 
  /* lang_hooks.tree_dump.dump_tree:  Dump language-specific parts of tree
     nodes.  Returns nonzero if it does not want the usual dumping of the
     second argument.  */
--- 295,300 ----
Index: gcc/langhooks.h
===================================================================
*** gcc.orig/langhooks.h	2007-07-26 17:19:36.000000000 +0200
--- gcc/langhooks.h	2007-07-26 17:23:59.000000000 +0200
*************** struct lang_hooks_for_tree_inlining
*** 37,43 ****
  {
    int (*cannot_inline_tree_fn) (tree *);
    int (*disregard_inline_limits) (tree);
-   int (*auto_var_in_fn_p) (tree, tree);
    bool (*var_mod_type_p) (tree, tree);
  };
  
--- 37,42 ----
Index: gcc/tree-inline.c
===================================================================
*** gcc.orig/tree-inline.c	2007-07-26 14:13:39.000000000 +0200
--- gcc/tree-inline.c	2007-07-26 17:23:36.000000000 +0200
*************** remap_decls (tree decls, copy_body_data 
*** 427,433 ****
        /* We can not chain the local static declarations into the unexpanded_var_list
           as we can't duplicate them or break one decl rule.  Go ahead and link
           them into unexpanded_var_list.  */
!       if (!lang_hooks.tree_inlining.auto_var_in_fn_p (old_var, id->src_fn)
  	  && !DECL_EXTERNAL (old_var))
  	{
  	  cfun->unexpanded_var_list = tree_cons (NULL_TREE, old_var,
--- 427,433 ----
        /* We can not chain the local static declarations into the unexpanded_var_list
           as we can't duplicate them or break one decl rule.  Go ahead and link
           them into unexpanded_var_list.  */
!       if (!auto_var_in_fn_p (old_var, id->src_fn)
  	  && !DECL_EXTERNAL (old_var))
  	{
  	  cfun->unexpanded_var_list = tree_cons (NULL_TREE, old_var,
*************** copy_body_r (tree *tp, int *walk_subtree
*** 585,591 ****
       variables.  We don't want to copy static variables; there's only
       one of those, no matter how many times we inline the containing
       function.  Similarly for globals from an outer function.  */
!   else if (lang_hooks.tree_inlining.auto_var_in_fn_p (*tp, fn))
      {
        tree new_decl;
  
--- 585,591 ----
       variables.  We don't want to copy static variables; there's only
       one of those, no matter how many times we inline the containing
       function.  Similarly for globals from an outer function.  */
!   else if (auto_var_in_fn_p (*tp, fn))
      {
        tree new_decl;
  
*************** copy_body_r (tree *tp, int *walk_subtree
*** 640,647 ****
  	 discarding.  */
        if (TREE_CODE (*tp) == GIMPLE_MODIFY_STMT
  	  && GIMPLE_STMT_OPERAND (*tp, 0) == GIMPLE_STMT_OPERAND (*tp, 1)
! 	  && (lang_hooks.tree_inlining.auto_var_in_fn_p
! 	      (GIMPLE_STMT_OPERAND (*tp, 0), fn)))
  	{
  	  /* Some assignments VAR = VAR; don't generate any rtl code
  	     and thus don't count as variable modification.  Avoid
--- 640,646 ----
  	 discarding.  */
        if (TREE_CODE (*tp) == GIMPLE_MODIFY_STMT
  	  && GIMPLE_STMT_OPERAND (*tp, 0) == GIMPLE_STMT_OPERAND (*tp, 1)
! 	  && (auto_var_in_fn_p (GIMPLE_STMT_OPERAND (*tp, 0), fn)))
  	{
  	  /* Some assignments VAR = VAR; don't generate any rtl code
  	     and thus don't count as variable modification.  Avoid
*************** self_inlining_addr_expr (tree value, tre
*** 1267,1273 ****
  
    var = get_base_address (TREE_OPERAND (value, 0));
  
!   return var && lang_hooks.tree_inlining.auto_var_in_fn_p (var, fn);
  }
  
  static void
--- 1266,1272 ----
  
    var = get_base_address (TREE_OPERAND (value, 0));
  
!   return var && auto_var_in_fn_p (var, fn);
  }
  
  static void
Index: gcc/tree.c
===================================================================
*** gcc.orig/tree.c	2007-07-26 17:19:36.000000000 +0200
--- gcc/tree.c	2007-07-26 17:21:32.000000000 +0200
*************** get_type_static_bounds (tree type, mpz_t
*** 6305,6310 ****
--- 6305,6323 ----
      }
  }
  
+ /* auto_var_in_fn_p is called to determine whether VAR is an automatic
+    variable defined in function FN.  */
+ 
+ bool
+ auto_var_in_fn_p (tree var, tree fn)
+ {
+   return (DECL_P (var) && DECL_CONTEXT (var) == fn
+ 	  && (((TREE_CODE (var) == VAR_DECL || TREE_CODE (var) == PARM_DECL)
+ 	       && ! TREE_STATIC (var))
+ 	      || TREE_CODE (var) == LABEL_DECL
+ 	      || TREE_CODE (var) == RESULT_DECL));
+ }
+ 
  /* Subprogram of following function.  Called by walk_tree.
  
     Return *TP if it is an automatic variable or parameter of the
*************** find_var_from_fn (tree *tp, int *walk_su
*** 6319,6325 ****
      *walk_subtrees = 0;
  
    else if (DECL_P (*tp)
! 	   && lang_hooks.tree_inlining.auto_var_in_fn_p (*tp, fn))
      return *tp;
  
    return NULL_TREE;
--- 6332,6338 ----
      *walk_subtrees = 0;
  
    else if (DECL_P (*tp)
! 	   && auto_var_in_fn_p (*tp, fn))
      return *tp;
  
    return NULL_TREE;
Index: gcc/tree.h
===================================================================
*** gcc.orig/tree.h	2007-07-26 17:19:36.000000000 +0200
--- gcc/tree.h	2007-07-26 17:22:51.000000000 +0200
*************** extern bool empty_body_p (tree);
*** 4390,4395 ****
--- 4390,4396 ----
  extern tree call_expr_arg (tree, int);
  extern tree *call_expr_argp (tree, int);
  extern tree call_expr_arglist (tree);
+ extern bool auto_var_in_fn_p (tree, tree);
  \f
  /* In stmt.c */
  

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Mark Mitchell]

Richard Guenther wrote:

> 2007-07-26  Richard Guenther  <rguenther@suse.de>
> 
> 	* langhooks-def.h (lhd_tree_inlining_auto_var_in_fn_p): Remove.
> 	(LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P): Likewise.
> 	(LANG_HOOKS_TREE_INLINING_INITIALIZER): Remove initializer for
> 	auto_var_in_fn_p langhook.
> 	* langhooks.c (lhd_tree_inlining_auto_var_in_fn_p): Rename and
> 	move ...
> 	* tree.c (auto_var_in_fn_p): ... here.
> 	(find_var_from_fn): Call auto_var_in_fn_p directly.
> 	* langhooks.h (lang_hooks_for_tree_inlining): Remove
> 	auto_var_in_fn_p langhook.
> 	* tree-inline.c (remap_decls): Call auto_var_in_fn_p directly.
> 	(copy_body_r): Likewise.
> 	(self_inlining_addr_expr): Likewise.
> 	* tree.h (auto_var_in_fn_p): Declare.
> 
> 	cp/
> 	* cp-objcp-common.h (LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P):
> 	Remove.
> 	* cp-tree.h (cp_auto_var_in_fn_p): Remove.
> 	* tree.c (cp_auto_var_in_fn_p): Remove.

OK.  I think this means nonstatic_local_decl_p is dead now too, so feel
free to remove that as well.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Richard Guenther]

On Wed, 1 Aug 2007, Mark Mitchell wrote:

> Richard Guenther wrote:
> 
> > 2007-07-26  Richard Guenther  <rguenther@suse.de>
> > 
> > 	* langhooks-def.h (lhd_tree_inlining_auto_var_in_fn_p): Remove.
> > 	(LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P): Likewise.
> > 	(LANG_HOOKS_TREE_INLINING_INITIALIZER): Remove initializer for
> > 	auto_var_in_fn_p langhook.
> > 	* langhooks.c (lhd_tree_inlining_auto_var_in_fn_p): Rename and
> > 	move ...
> > 	* tree.c (auto_var_in_fn_p): ... here.
> > 	(find_var_from_fn): Call auto_var_in_fn_p directly.
> > 	* langhooks.h (lang_hooks_for_tree_inlining): Remove
> > 	auto_var_in_fn_p langhook.
> > 	* tree-inline.c (remap_decls): Call auto_var_in_fn_p directly.
> > 	(copy_body_r): Likewise.
> > 	(self_inlining_addr_expr): Likewise.
> > 	* tree.h (auto_var_in_fn_p): Declare.
> > 
> > 	cp/
> > 	* cp-objcp-common.h (LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P):
> > 	Remove.
> > 	* cp-tree.h (cp_auto_var_in_fn_p): Remove.
> > 	* tree.c (cp_auto_var_in_fn_p): Remove.
> 
> OK.  I think this means nonstatic_local_decl_p is dead now too, so feel
> free to remove that as well.

Right.  Done.

Thanks,
Richard.

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Jakub Jelinek]

On Mon, Aug 20, 2007 at 01:32:11PM +0200, Richard Guenther wrote:
> On Wed, 1 Aug 2007, Mark Mitchell wrote:
> 
> > Richard Guenther wrote:
> > 
> > > 2007-07-26  Richard Guenther  <rguenther@suse.de>
> > > 
> > > 	* langhooks-def.h (lhd_tree_inlining_auto_var_in_fn_p): Remove.
> > > 	(LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P): Likewise.
> > > 	(LANG_HOOKS_TREE_INLINING_INITIALIZER): Remove initializer for
> > > 	auto_var_in_fn_p langhook.
> > > 	* langhooks.c (lhd_tree_inlining_auto_var_in_fn_p): Rename and
> > > 	move ...
> > > 	* tree.c (auto_var_in_fn_p): ... here.
> > > 	(find_var_from_fn): Call auto_var_in_fn_p directly.
> > > 	* langhooks.h (lang_hooks_for_tree_inlining): Remove
> > > 	auto_var_in_fn_p langhook.
> > > 	* tree-inline.c (remap_decls): Call auto_var_in_fn_p directly.
> > > 	(copy_body_r): Likewise.
> > > 	(self_inlining_addr_expr): Likewise.
> > > 	* tree.h (auto_var_in_fn_p): Declare.
> > > 
> > > 	cp/
> > > 	* cp-objcp-common.h (LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P):
> > > 	Remove.
> > > 	* cp-tree.h (cp_auto_var_in_fn_p): Remove.
> > > 	* tree.c (cp_auto_var_in_fn_p): Remove.
> > 
> > OK.  I think this means nonstatic_local_decl_p is dead now too, so feel
> > free to remove that as well.
> 
> Right.  Done.

One of the -r127641:127644 changes causes
FAIL: gcc.c-torture/execute/20010119-1.c compilation,  -Os
regression on x86_64-linux.  The inline fn is no longer inlined and as it
is extern inline in gnu89 mode, we get undefined reference.
In this particular case inlining it is a net win even for -Os, as it is
completely optimized out.

	Jakub

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Richard Guenther]

On Tue, 21 Aug 2007, Jakub Jelinek wrote:

> On Mon, Aug 20, 2007 at 01:32:11PM +0200, Richard Guenther wrote:
> > On Wed, 1 Aug 2007, Mark Mitchell wrote:
> > 
> > > Richard Guenther wrote:
> > > 
> > > > 2007-07-26  Richard Guenther  <rguenther@suse.de>
> > > > 
> > > > 	* langhooks-def.h (lhd_tree_inlining_auto_var_in_fn_p): Remove.
> > > > 	(LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P): Likewise.
> > > > 	(LANG_HOOKS_TREE_INLINING_INITIALIZER): Remove initializer for
> > > > 	auto_var_in_fn_p langhook.
> > > > 	* langhooks.c (lhd_tree_inlining_auto_var_in_fn_p): Rename and
> > > > 	move ...
> > > > 	* tree.c (auto_var_in_fn_p): ... here.
> > > > 	(find_var_from_fn): Call auto_var_in_fn_p directly.
> > > > 	* langhooks.h (lang_hooks_for_tree_inlining): Remove
> > > > 	auto_var_in_fn_p langhook.
> > > > 	* tree-inline.c (remap_decls): Call auto_var_in_fn_p directly.
> > > > 	(copy_body_r): Likewise.
> > > > 	(self_inlining_addr_expr): Likewise.
> > > > 	* tree.h (auto_var_in_fn_p): Declare.
> > > > 
> > > > 	cp/
> > > > 	* cp-objcp-common.h (LANG_HOOKS_TREE_INLINING_AUTO_VAR_IN_FN_P):
> > > > 	Remove.
> > > > 	* cp-tree.h (cp_auto_var_in_fn_p): Remove.
> > > > 	* tree.c (cp_auto_var_in_fn_p): Remove.
> > > 
> > > OK.  I think this means nonstatic_local_decl_p is dead now too, so feel
> > > free to remove that as well.
> > 
> > Right.  Done.
> 
> One of the -r127641:127644 changes causes
> FAIL: gcc.c-torture/execute/20010119-1.c compilation,  -Os
> regression on x86_64-linux.  The inline fn is no longer inlined and as it
> is extern inline in gnu89 mode, we get undefined reference.
> In this particular case inlining it is a net win even for -Os, as it is
> completely optimized out.

I didn't see this failure with the original testing.  This is caused by
the disregard_inline_limits langhook removal where I changed the C 
frontend to not disregard inline limits for extern inline functions.
For -Os heuristics now decide that it is not profitable to inline this
function.  As extern inline with gnu semantics says that there is an
extern instance of this function we get a link time error for this
testcase at -Os.

Considering foo with 23 insns
 to be inlined into main
 Estimated growth after inlined into all callees is +5 insns.
 Estimated badness is 17, frequency 1.00.
 inline_failed:call is unlikely.

(16 of the cost is because of our atm artificially high call cost
which is accounted to the call to undef(), the rest is the single
addition and the 3 conditionals with their comparisons)

I suppose we should disable the test for -Os or wait for the inliner
tunings (we have another PR about the high call cost, experiments showed
8 is a more reasonable value which would fix this testcase as well, but
the tuning waits for the early DSE work).

Richard.

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Richard Guenther]

On Tue, 21 Aug 2007, Richard Guenther wrote:

> On Tue, 21 Aug 2007, Jakub Jelinek wrote:
> > One of the -r127641:127644 changes causes
> > FAIL: gcc.c-torture/execute/20010119-1.c compilation,  -Os
> > regression on x86_64-linux.  The inline fn is no longer inlined and as it
> > is extern inline in gnu89 mode, we get undefined reference.
> > In this particular case inlining it is a net win even for -Os, as it is
> > completely optimized out.
> 
> I didn't see this failure with the original testing.  This is caused by
> the disregard_inline_limits langhook removal where I changed the C 
> frontend to not disregard inline limits for extern inline functions.
> For -Os heuristics now decide that it is not profitable to inline this
> function.  As extern inline with gnu semantics says that there is an
> extern instance of this function we get a link time error for this
> testcase at -Os.
> 
> Considering foo with 23 insns
>  to be inlined into main
>  Estimated growth after inlined into all callees is +5 insns.
>  Estimated badness is 17, frequency 1.00.
>  inline_failed:call is unlikely.
> 
> (16 of the cost is because of our atm artificially high call cost
> which is accounted to the call to undef(), the rest is the single
> addition and the 3 conditionals with their comparisons)
> 
> I suppose we should disable the test for -Os or wait for the inliner
> tunings (we have another PR about the high call cost, experiments showed
> 8 is a more reasonable value which would fix this testcase as well, but
> the tuning waits for the early DSE work).

Note we also may preserve previous behavior with something like

Index: tree-inline.c
===================================================================
*** tree-inline.c	(revision 127658)
--- tree-inline.c	(working copy)
*************** inlinable_function_p (tree fn)
*** 1920,1925 ****
--- 1920,1930 ----
  bool
  disregard_inline_limits_p (tree fn)
  {
+   /* GNU extern inline functions are supposed to be cheap.  */
+   if (DECL_DECLARED_INLINE_P (fn)
+       && DECL_EXTERNAL (fn))
+     return true;
+ 
    return lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) != NULL_TREE;
  }

(supposed that DECL_DECLARED_INLINE_P and DECL_EXTERNAL is not used with
different semantics in other frontends).  But I think this is the wrong
thing for -Os.

Richard.

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Jakub Jelinek]

On Tue, Aug 21, 2007 at 11:46:39AM +0200, Richard Guenther wrote:
> I didn't see this failure with the original testing.  This is caused by
> the disregard_inline_limits langhook removal where I changed the C 
> frontend to not disregard inline limits for extern inline functions.
> For -Os heuristics now decide that it is not profitable to inline this
> function.  As extern inline with gnu semantics says that there is an
> extern instance of this function we get a link time error for this
> testcase at -Os.
> 
> Considering foo with 23 insns
>  to be inlined into main
>  Estimated growth after inlined into all callees is +5 insns.
>  Estimated badness is 17, frequency 1.00.
>  inline_failed:call is unlikely.
> 
> (16 of the cost is because of our atm artificially high call cost
> which is accounted to the call to undef(), the rest is the single
> addition and the 3 conditionals with their comparisons)
> 
> I suppose we should disable the test for -Os or wait for the inliner
> tunings (we have another PR about the high call cost, experiments showed
> 8 is a more reasonable value which would fix this testcase as well, but
> the tuning waits for the early DSE work).

Guess
-#ifdef __OPTIMIZE__
+#if defined __OPTIMIZE__ && !defined __OPTIMIZE_SIZE__
could be used for now with a comment, better than adding *.x
and doing tcl hacks in it.

	Jakub

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Jakub Jelinek]

On Tue, Aug 21, 2007 at 12:02:22PM +0200, Richard Guenther wrote:
> Note we also may preserve previous behavior with something like
> 
> Index: tree-inline.c
> ===================================================================
> *** tree-inline.c	(revision 127658)
> --- tree-inline.c	(working copy)
> *************** inlinable_function_p (tree fn)
> *** 1920,1925 ****
> --- 1920,1930 ----
>   bool
>   disregard_inline_limits_p (tree fn)
>   {
> +   /* GNU extern inline functions are supposed to be cheap.  */
> +   if (DECL_DECLARED_INLINE_P (fn)
> +       && DECL_EXTERNAL (fn))
> +     return true;
> + 
>     return lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) != NULL_TREE;
>   }
> 
> (supposed that DECL_DECLARED_INLINE_P and DECL_EXTERNAL is not used with
> different semantics in other frontends).  But I think this is the wrong
> thing for -Os.

That would be my preference actually, while it is perhaps not documented,
that's how gcc behaved for years and a lot of code relies on it.
Or bias the results of the heuristics if GNU89 extern inline.
The typical uses of extern inline are very small routines which often
use a lot of __builtin_constant_p, and as you could see even on the
20010119-1.c testcase the heuristics is often wrong.
glibc headers these days uses always_inline when it is absolutely necessary
to inline, but where it is not, but it is a very desirable optimization,
it will still use extern inline.
Consider e.g.
#include <wchar.h>

int
foo (void)
{
  return btowc (L' ');
}

With -Os before your patch this generates on x86_64
foo:	movl    $32, %eax
        ret
after the patch:
foo:    movl    $32, %edi
        jmp     btowc
Bigger and far slower.  I bet I can come up with countless other examples.

	Jakub

Re: [PATCH][C++] Remove auto_var_in_fn langhook

[Richard Guenther]

On Tue, 21 Aug 2007, Jakub Jelinek wrote:

> On Tue, Aug 21, 2007 at 12:02:22PM +0200, Richard Guenther wrote:
> > Note we also may preserve previous behavior with something like
> > 
> > Index: tree-inline.c
> > ===================================================================
> > *** tree-inline.c	(revision 127658)
> > --- tree-inline.c	(working copy)
> > *************** inlinable_function_p (tree fn)
> > *** 1920,1925 ****
> > --- 1920,1930 ----
> >   bool
> >   disregard_inline_limits_p (tree fn)
> >   {
> > +   /* GNU extern inline functions are supposed to be cheap.  */
> > +   if (DECL_DECLARED_INLINE_P (fn)
> > +       && DECL_EXTERNAL (fn))
> > +     return true;
> > + 
> >     return lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) != NULL_TREE;
> >   }
> > 
> > (supposed that DECL_DECLARED_INLINE_P and DECL_EXTERNAL is not used with
> > different semantics in other frontends).  But I think this is the wrong
> > thing for -Os.
> 
> That would be my preference actually, while it is perhaps not documented,
> that's how gcc behaved for years and a lot of code relies on it.
> Or bias the results of the heuristics if GNU89 extern inline.
> The typical uses of extern inline are very small routines which often
> use a lot of __builtin_constant_p, and as you could see even on the
> 20010119-1.c testcase the heuristics is often wrong.
> glibc headers these days uses always_inline when it is absolutely necessary
> to inline, but where it is not, but it is a very desirable optimization,
> it will still use extern inline.
> Consider e.g.
> #include <wchar.h>
> 
> int
> foo (void)
> {
>   return btowc (L' ');
> }
> 
> With -Os before your patch this generates on x86_64
> foo:	movl    $32, %eax
>         ret
> after the patch:
> foo:    movl    $32, %edi
>         jmp     btowc
> Bigger and far slower.  I bet I can come up with countless other examples.

Ok, I'll test the above patch.

Richard.