Re: Fix C/C++ inlinability tests

[Steven Bosscher <s dot bosscher at student dot tudelft dot nl>]

Hi,


Op di 09-09-2003, om 20:44 schreef Jan Hubicka:
> The patch moves checks that will become completely frontend independent after
> gimplifying into tree-inline (at the present there is one ifdef needed) and
> cuts number of walks trought body from three into one.

Cool.

> It also improves warnings on why inlining failed.

Cool, too :-)

> OK?
> Honza
> Tue Sep  9 20:32:37 CEST 2003  Jan Hubicka  <jh@suse.cz>
> 	* c-objc-common.c (c_cannot_inline_tree_fn): Warn
> 	on why function is not inlinable; do not check
> 	the body.
> 	(inline_forbidden_p): Move to...
... where? Oh, tree-inline.c...

> 	* tree-inline.c (inline_forbidden_p_1): Add warnings;
> 	deal with alloca, longjmp.
> 	(inline_forbidden_p): New static function.
> 	(find_alloca_call_1, find_alloca_call, find_builtin_longjmp_call_1,
> 	find_builtin_longjmp_call): Kill.

> --- 64,108 ----
>     return DECL_DECLARED_INLINE_P (fn) && DECL_EXTERNAL (fn);
>   }
>   
>   int
>   c_cannot_inline_tree_fn (tree *fnp)
>   {
>     tree fn = *fnp;
>     tree t;
> +   bool do_warning = (warn_inline
> + 		     && DECL_INLINE (fn)
> + 		     && DECL_DECLARED_INLINE_P (fn)
> + 		     && !DECL_IN_SYSTEM_HEADER (fn));
>   
>     if (flag_really_no_inline
> 	&& lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)) == NULL)
> !     {
> !       if (do_warning)
> ! 	warning ("%Hfunction '%F' can never be inlined because it "
> ! 		 "is supressed using -fno-really-inline",

This flag does not really exist.  It's another piece of uglyness that
can disappear when flag_inline_trees finally dies (which requires that
Java stops using the RTL inliner first, sigh). You really want to use
"-fno-inline" in the warning.

> ! 		 &DECL_SOURCE_LOCATION (fn), fn);
> !       return 1;
> !     }

So this will warn for every function? I think one warning is enough, I
suppose someone who uses -fno-inline.

  
>     /* Don't auto-inline anything that might not be bound within
>        this unit of translation.  */
>     if (!DECL_DECLARED_INLINE_P (fn) && !(*targetm.binds_local_p) (fn))
> !     {
> !       if (do_warning)
> ! 	warning ("%Hfunction '%F' can never be inlined because it might not "
> ! 		 "be bound within this unit of translation",
> ! 		 &DECL_SOURCE_LOCATION (fn), fn);
> !       goto cannot_inline;
> !     }
>   
>     if (! function_attribute_inlinable_p (fn))
> !     {
> !       if (do_warning)
> ! 	warning ("%Hfunction '%F' can never be inlined because it use "
									^
should be "... it uses "

> ! 		 "attributes conflicting with inlining",
> ! 		 &DECL_SOURCE_LOCATION (fn), fn);
> !       goto cannot_inline;
> !     }
>   
>     /* If a function has pending sizes, we must not defer its
>        compilation, and we can't inline it as a tree.  */
> *************** c_cannot_inline_tree_fn (tree *fnp)
> *** 178,184 ****
> 	put_pending_sizes (t);
>   
> 	if (t)
> ! 	goto cannot_inline;
>       }
>   
>     if (! C_DECL_FILE_SCOPE (fn))
> --- 112,124 ----
> 	put_pending_sizes (t);
>   
> 	if (t)
> ! 	{
> ! 	  if (do_warning)
> ! 	    warning ("%Hfunction '%F' can never be inlined because it has "
> ! 		     "pending sizes",
> ! 		     &DECL_SOURCE_LOCATION (fn), fn);

Is Joe User going to have a clue what you mean with pending sizes?

> ! 	  goto cannot_inline;
> ! 	}
>       }
>   
>     if (! C_DECL_FILE_SCOPE (fn))
> *************** c_cannot_inline_tree_fn (tree *fnp)
> *** 186,216 ****
---- 8< ----
> --- 126,140 ----
> 	/* If a nested function has pending sizes, we may have already
> 	   saved them.  */
> 	if (DECL_LANG_SPECIFIC (fn)->pending_sizes)
> ! 	{
> ! 	  if (do_warning)
> ! 	    warning ("%Hnested function '%F' can never be inlined because it "
> ! 		     "has possibly saved pending sizes",

Same here.

> ! 		     &DECL_SOURCE_LOCATION (fn), fn);
> ! 	  goto cannot_inline;
> ! 	}
>       }
>   
>     return 0;
>   
>    cannot_inline:
> 
> diff -Nrc3p gcc.old3/tree-inline.c gcc/tree-inline.c
> *** gcc.old3/tree-inline.c	Tue Sep  9 17:06:38 2003
> --- gcc/tree-inline.c	Tue Sep  9 20:22:54 2003
> *************** static tree initialize_inlined_parameter
> *** 128,137 ****
>   static void remap_block (tree *, tree, inline_data *);
>   static tree add_stmt_to_compound (tree, tree, tree);
>   #endif /* INLINER_FOR_JAVA */
> - static tree find_alloca_call_1 (tree *, int *, void *);
> - static tree find_alloca_call (tree);
> - static tree find_builtin_longjmp_call_1 (tree *, int *, void *);
> - static tree find_builtin_longjmp_call (tree);

Hubba hubba. That was on my TODO list :-)
 
> --- 876,1022 ----
>     return inlinable_function_p (fn);
>   }
>   
> ! static const char *inline_forbidden_reason;
>   
>   static tree
> ! inline_forbidden_p_1 (tree *nodep, int *walk_subtrees ATTRIBUTE_UNUSED,
> ! 		    void *fn)
>   {
> !   tree node = *nodep;
> !   tree t;
>   
> !   switch (TREE_CODE (node))
> !     {
> !     case CALL_EXPR:
> !       /* Refuse to inline alloca call unless user explicitly forced so as this
> ! 	 may change program's memory overhead drastically when the function
> ! 	 using alloca is called in loop.  In GCC present in SPEC2000 inlining
> ! 	 into schedule_block cause it to require 2GB of ram instead of 256MB.  */
> !       if (alloca_call_p (node) && !lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)))
> ! 	{
> ! 	  inline_forbidden_reason = "%Hfunction '%F' can never be inlined "
> ! 				    "because it use alloca (overwrite using "

"it uses" again.

> ! 				    "always_inline attribute)";
> ! 	  return node;
> ! 	}
> !       t = get_callee_fndecl (node);
> !       if (! t)
> ! 	break;
>   
>   
> !       /* We cannot inline functions that call setjmp.  */
> !       if (setjmp_call_p (t))
> ! 	{
> ! 	  inline_forbidden_reason = "%Hfunction '%F' can never be inlined"
> ! 				    " because it use setjmp";
> ! 	  return node;
> ! 	}

I had a patch at some point to move this check from the C/C++ front ends
to tree-inline, but it wouldn't work with C++. The C++ front end also
checks for setjmp, but way earlier for some reason (something with
templates not yet being available when the check for setjmp happend,
IIRC -- but it's been a while...). You're not seeing any trouble with
your setjmp check here?

> 
> ! 
> !       switch (DECL_FUNCTION_CODE (t))
> ! 	{
> ! 	  /* We cannot inline functions that take a variable number of
> ! 	     arguments.  */
> ! 	case BUILT_IN_VA_START:
> ! 	case BUILT_IN_STDARG_START:
> ! 	  {
> ! 	    inline_forbidden_reason = "%Hfunction '%F' can never be inlined"
> ! 				      " because it use variable argument lists";

... another "it uses".


> ! 	    return node;
> ! 	  }
> ! 	case BUILT_IN_LONGJMP:
> ! #ifndef INLINER_FOR_JAVA
> !     case DECL_STMT:
> !       /* We cannot inline functions that contain other functions.  */
> !       if (TREE_CODE (TREE_OPERAND (node, 0)) == FUNCTION_DECL
> ! 	  && DECL_INITIAL (TREE_OPERAND (node, 0)))
> ! 	{
> ! 	  inline_forbidden_reason = "%Hfunction '%F' can never be inlined "
> ! 				    "because it contains nested function";
> ! 	  return node;
> ! 	}
> !       break;

For some time I've been wondering: is this still true if all nested
function calls are themselves tree-inlined, and would there any way to
make sure that this is the case?

> !     case GOTO_STMT:
> !     case GOTO_EXPR:
> !       t = TREE_OPERAND (node, 0);
> ! 
> !       /* We will not inline a function which uses computed goto.  The
> ! 	 addresses of its local labels, which may be tucked into
> ! 	 global storage, are of course not constant across
> ! 	 instantiations, which causes unexpected behavior.  */
> !       if (TREE_CODE (t) != LABEL_DECL)
> ! 	{
> ! 	  inline_forbidden_reason = "%Hfunction '%F' can never be inlined "
> ! 				    "because it contains nonlocal label";
							  ^

"a nonlocal label", or "one or more local labels"

> ! 	  return node;
> ! 	}
> ! 
> !       /* We cannot inline a nested function that jumps to a nonlocal
> !          label.  */
> !       if (TREE_CODE (t) == LABEL_DECL && DECL_CONTEXT (t) != fn)
> ! 	{
> ! 	  inline_forbidden_reason = "%Hfunction '%F' can never be inlined "
> ! 				    " because it contains nonlocal goto";
							   ^

same...

> ! 	  return node;
> ! 	}
> ! 
> !       break;
> ! 
> !     case RECORD_TYPE:
> !     case UNION_TYPE:
> !       /* We cannot inline a function of the form
> ! 
> ! 	   void F (int i) { struct S { int ar[i]; } s; }
> ! 
> ! 	 Attempting to do so produces a catch-22 in tree-inline.c.

Hmm the patch is big enough that I'm not sure, but isn't _this_ code in
tree-inline.c now? :-)

> ! 	 If walk_tree examines the TYPE_FIELDS chain of RECORD_TYPE/
> ! 	 UNION_TYPE nodes, then it goes into infinite recursion on a
> ! 	 structure containing a pointer to its own type.  If it doesn't,
> ! 	 then the type node for S doesn't get adjusted properly when
> ! 	 F is inlined, and we abort in find_function_data.  */
> !       for (t = TYPE_FIELDS (node); t; t = TREE_CHAIN (t))
> ! 	if (variably_modified_type_p (TREE_TYPE (t)))
> ! 	  {
> ! 	    inline_forbidden_reason = "%Hfunction '%F' can never be inlined "
> ! 				      "because it use variable sized variables";
> ! 	    return node;
> ! 	  }
> ! #endif



Gr.
Steven