Re: Fix gcc.c-torture/execute/eeprof-1.c with linker plugin

[Richard Guenther <rguenther at suse dot de>]

On Tue, 30 Nov 2010, Jan Hubicka wrote:

> Hi,
> this patch fixed eeprof-1 testcase with linker plugin enabled (or with -fwhole-program). 
> The testcases tests that profiling hooks __cyg_profile_func_enter
> and __cyg_profile_func_exit are properly called at start and end of every function.
> 
> This is implemented via builtin functions BUILT_IN_PROFILE_FUNC_ENTER and
> BUILT_IN_PROFILE_FUNC_EXIT that at RTL expansion time are translated into calls
> to __cyg_profile_func_enter (&current_function, __builtin_return_address())
> 
> The problem is that with linker plugin we bring __cyg_profile_func_exit and
> __cyg_profile_func_enter local and until the builtins are expanded we se no use
> of them so they get removed from the program.
> 
> I do not see why the code is organized this way and why we don't just insert the
> calls to profiling hooks directly at gimplification.  This patch does that - it
> makes __cyg_profile_func_exit and __cyg_profile_func_enter bultins, removes their
> specialized expansion and instead adds all the logic into gimplifier.
> 
> Seems to make sense?

Yes, but ...

> Honza
> 
> 	* tree.c (build_common_builtin_nodes): Do not initialize
> 	BUILT_IN_PROFILE_FUNC_ENTER and BUILT_IN_PROFILE_FUNC_EXIT.
> 	* builtins.c (expand_builtin_profile_func): Remove.
> 	(expand_builtin): Do not handle BUILT_IN_PROFILE_FUNC_ENTER and
> 	BUILT_IN_PROFILE_FUNC_EXIT.
> 	* builtins.def (profile_func_enter, profile_func_exit): Remove stubs.
> 	(__cyg_profile_func_enter, __cyg_profile_func_exit): New.
> 	* gimplify.c (gimplify_function_tree): Reorganize code calling
> 	profiling functions.
> Index: tree.c
> ===================================================================
> *** tree.c	(revision 167242)
> --- tree.c	(working copy)
> *************** build_common_builtin_nodes (void)
> *** 9324,9335 ****
>   			BUILT_IN_STACK_RESTORE,
>   			"__builtin_stack_restore", ECF_NOTHROW | ECF_LEAF);
>   
> -   ftype = build_function_type_list (void_type_node, NULL_TREE);
> -   local_define_builtin ("__builtin_profile_func_enter", ftype,
> - 			BUILT_IN_PROFILE_FUNC_ENTER, "profile_func_enter", 0);
> -   local_define_builtin ("__builtin_profile_func_exit", ftype,
> - 			BUILT_IN_PROFILE_FUNC_EXIT, "profile_func_exit", 0);
> - 
>     /* If there's a possibility that we might use the ARM EABI, build the
>       alternate __cxa_end_cleanup node used to resume from C++ and Java.  */
>     if (targetm.arm_eabi_unwinder)
> --- 9324,9329 ----
> Index: builtins.c
> ===================================================================
> *** builtins.c	(revision 167242)
> --- builtins.c	(working copy)
> *************** build_string_literal (int len, const cha
> *** 5203,5232 ****
>     return t;
>   }
>   
> - /* Expand a call to either the entry or exit function profiler.  */
> - 
> - static rtx
> - expand_builtin_profile_func (bool exitp)
> - {
> -   rtx this_rtx, which;
> - 
> -   this_rtx = DECL_RTL (current_function_decl);
> -   gcc_assert (MEM_P (this_rtx));
> -   this_rtx = XEXP (this_rtx, 0);
> - 
> -   if (exitp)
> -     which = profile_function_exit_libfunc;
> -   else
> -     which = profile_function_entry_libfunc;
> - 
> -   emit_library_call (which, LCT_NORMAL, VOIDmode, 2, this_rtx, Pmode,
> - 		     expand_builtin_return_addr (BUILT_IN_RETURN_ADDRESS,
> - 						 0),
> - 		     Pmode);
> - 
> -   return const0_rtx;
> - }
> - 
>   /* Expand a call to __builtin___clear_cache.  */
>   
>   static rtx
> --- 5203,5208 ----
> *************** expand_builtin (tree exp, rtx target, rt
> *** 6365,6375 ****
>         expand_builtin_prefetch (exp);
>         return const0_rtx;
>   
> -     case BUILT_IN_PROFILE_FUNC_ENTER:
> -       return expand_builtin_profile_func (false);
> -     case BUILT_IN_PROFILE_FUNC_EXIT:
> -       return expand_builtin_profile_func (true);
> - 
>       case BUILT_IN_INIT_TRAMPOLINE:
>         return expand_builtin_init_trampoline (exp);
>       case BUILT_IN_ADJUST_TRAMPOLINE:
> --- 6341,6346 ----
> Index: builtins.def
> ===================================================================
> *** builtins.def	(revision 167242)
> --- builtins.def	(working copy)
> *************** DEF_EXT_LIB_BUILTIN    (BUILT_IN_VFPRINT
> *** 743,750 ****
>   DEF_EXT_LIB_BUILTIN    (BUILT_IN_VPRINTF_CHK, "__vprintf_chk", BT_FN_INT_INT_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_2_0)
>   
>   /* Profiling hooks.  */
> ! DEF_BUILTIN_STUB (BUILT_IN_PROFILE_FUNC_ENTER, "profile_func_enter")
> ! DEF_BUILTIN_STUB (BUILT_IN_PROFILE_FUNC_EXIT, "profile_func_exit")
>   
>   /* TLS emulation.  */
>   DEF_BUILTIN (BUILT_IN_EMUTLS_GET_ADDRESS, targetm.emutls.get_address,
> --- 743,752 ----
>   DEF_EXT_LIB_BUILTIN    (BUILT_IN_VPRINTF_CHK, "__vprintf_chk", BT_FN_INT_INT_CONST_STRING_VALIST_ARG, ATTR_FORMAT_PRINTF_2_0)
>   
>   /* Profiling hooks.  */
> ! DEF_BUILTIN (BUILT_IN_PROFILE_FUNC_ENTER, "__cyg_profile_func_enter", BUILT_IN_NORMAL, BT_FN_VOID_PTR_PTR, BT_LAST,
> ! 	     false, false, false, ATTR_NULL, true, true)
> ! DEF_BUILTIN (BUILT_IN_PROFILE_FUNC_EXIT, "__cyg_profile_func_exit", BUILT_IN_NORMAL, BT_FN_VOID_PTR_PTR, BT_LAST,
> ! 	     false, false, false, ATTR_NULL, true, true)
>   
>   /* TLS emulation.  */
>   DEF_BUILTIN (BUILT_IN_EMUTLS_GET_ADDRESS, targetm.emutls.get_address,
> Index: gimplify.c
> ===================================================================
> *** gimplify.c	(revision 167242)
> --- gimplify.c	(working copy)
> *************** gimplify_function_tree (tree fndecl)
> *** 7862,7874 ****
>         gimple new_bind;
>         gimple tf;
>         gimple_seq cleanup = NULL, body = NULL;
>   
>         x = implicit_built_in_decls[BUILT_IN_PROFILE_FUNC_EXIT];
> !       gimplify_seq_add_stmt (&cleanup, gimple_build_call (x, 0));
>         tf = gimple_build_try (seq, cleanup, GIMPLE_TRY_FINALLY);
>   
>         x = implicit_built_in_decls[BUILT_IN_PROFILE_FUNC_ENTER];
> !       gimplify_seq_add_stmt (&body, gimple_build_call (x, 0));
>         gimplify_seq_add_stmt (&body, tf);
>         new_bind = gimple_build_bind (NULL, body, gimple_bind_block (bind));
>         /* Clear the block for BIND, since it is no longer directly inside
> --- 7862,7894 ----
>         gimple new_bind;
>         gimple tf;
>         gimple_seq cleanup = NULL, body = NULL;
> +       tree tmp_var;
> +       gimple call;
>   
> +       x = implicit_built_in_decls[BUILT_IN_RETURN_ADDRESS];
> +       call = gimple_build_call (x, 0);
> +       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
> +       gimple_call_set_lhs (call, tmp_var);
> +       gimplify_seq_add_stmt (&cleanup, call);
>         x = implicit_built_in_decls[BUILT_IN_PROFILE_FUNC_EXIT];
> !       gimplify_seq_add_stmt (&cleanup,
> ! 			     gimple_build_call
> ! 			       (x, 2,
> ! 				build_fold_addr_expr (current_function_decl),
> ! 				tmp_var));

Put that call expr to the call temporary as well to avoid funny
indents.

>         tf = gimple_build_try (seq, cleanup, GIMPLE_TRY_FINALLY);
>   
> +       x = implicit_built_in_decls[BUILT_IN_RETURN_ADDRESS];
> +       call = gimple_build_call (x, 0);
> +       tmp_var = create_tmp_var (ptr_type_node, "return_addr");
> +       gimple_call_set_lhs (call, tmp_var);
> +       gimplify_seq_add_stmt (&body, call);
>         x = implicit_built_in_decls[BUILT_IN_PROFILE_FUNC_ENTER];
> !       gimplify_seq_add_stmt (&body,
> ! 			     gimple_build_call
> ! 			       (x, 2,
> ! 				build_fold_addr_expr (current_function_decl),
> ! 				tmp_var));

Likewise.

>         gimplify_seq_add_stmt (&body, tf);
>         new_bind = gimple_build_bind (NULL, body, gimple_bind_block (bind));
>         /* Clear the block for BIND, since it is no longer directly inside
> 
> 

Ok with that change.

Thanks,
Richard.