[Patch/cfgexpand] : also consider assembler_name to call expand_main_function

On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:

> On Wed, 14 Mar 2012, Tristan Gingold wrote:
[…]

> 
> Well.  To make this work in LTO the "main" function (thus, the program
> entry point) should be marked at cgraph level and all users of
> MAIN_NAME_P should instead check a flag on the cgraph node.
> 
>> Will write a predicate in tree.[ch].
> 
> Please instead transition "main-ness" to the graph.

Hi,

here is the patch I wrote.  Does it match what you had in mind ?

main_identifier_node is now set in tree.c

I haven't changed MAIN_NAME_P uses in c-decl.c and cp/decl.c (obviously).
I haven't yet checked beyond simple build.

Tristan.

On Tue, 20 Mar 2012, Tristan Gingold wrote:

> 
> On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:
> 
> > On Wed, 14 Mar 2012, Tristan Gingold wrote:
> […]
> 
> > 
> > Well.  To make this work in LTO the "main" function (thus, the program
> > entry point) should be marked at cgraph level and all users of
> > MAIN_NAME_P should instead check a flag on the cgraph node.
> > 
> >> Will write a predicate in tree.[ch].
> > 
> > Please instead transition "main-ness" to the graph.
> 
> Hi,
> 
> here is the patch I wrote.  Does it match what you had in mind ?

Basically yes.  Comments below.

> main_identifier_node is now set in tree.c

Looks good, hopefully my review-grep was as good as yours ;)

> I haven't changed MAIN_NAME_P uses in c-decl.c and cp/decl.c (obviously).
> I haven't yet checked beyond simple build.
> 
> Tristan.
> 
> diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
> index 89f5438..c575e97 100644
> --- a/gcc/ada/gcc-interface/trans.c
> +++ b/gcc/ada/gcc-interface/trans.c
> @@ -622,8 +622,6 @@ gigi (Node_Id gnat_root, int max_gnat_node, int number_name ATTRIBUTE_UNUSED,
>  		       integer_type_node, NULL_TREE, true, false, true, false,
>  		       NULL, Empty);
>  
> -  main_identifier_node = get_identifier ("main");
> -
>    /* Install the builtins we might need, either internally or as
>       user available facilities for Intrinsic imports.  */
>    gnat_install_builtins ();
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index b83f45b..5d05d8a 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -5094,8 +5094,6 @@ c_common_nodes_and_builtins (void)
>    if (!flag_preprocess_only)
>      c_define_builtins (va_list_ref_type_node, va_list_arg_type_node);
>  
> -  main_identifier_node = get_identifier ("main");
> -
>    /* Create the built-in __null node.  It is important that this is
>       not shared.  */
>    null_node = make_node (INTEGER_CST);
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index bd21169..7a7a774 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void)
>  
>    /* If this function is `main', emit a call to `__main'
>       to run global initializers, etc.  */
> -  if (DECL_NAME (current_function_decl)
> -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
> -      && DECL_FILE_SCOPE_P (current_function_decl))
> +  if (DECL_FILE_SCOPE_P (current_function_decl)
> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
>      expand_main_function ();

The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere
you call cgraph_main_function_p.  I suppose returning false if the
cgraph node is NULL in cgraph_main_function_p would be good.

>  
>    /* Initialize the stack_protect_guard field.  This must happen after the
> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
> index 9cc3690..528fd19 100644
> --- a/gcc/cgraph.c
> +++ b/gcc/cgraph.c
> @@ -2766,7 +2766,7 @@ cgraph_propagate_frequency_1 (struct cgraph_node *node, void *data)
>  	  /* It makes sense to put main() together with the static constructors.
>  	     It will be executed for sure, but rest of functions called from
>  	     main are definitely not at startup only.  */
> -	  if (MAIN_NAME_P (DECL_NAME (edge->caller->decl)))
> +	  if (cgraph_main_function_p (edge->caller))
>  	    d->only_called_at_startup = 0;
>            d->only_called_at_exit &= edge->caller->only_called_at_exit;
>  	}
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index 191364c..4db3417 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -101,6 +101,9 @@ struct GTY(()) cgraph_local_info {
>  
>    /* True if the function may enter serial irrevocable mode.  */
>    unsigned tm_may_enter_irr : 1;
> +
> +  /* True if the function is the program entry point (main in C).  */
> +  unsigned main_function : 1;
>  };
>  
>  /* Information about the function that needs to be computed globally
> @@ -790,6 +793,13 @@ cgraph_next_function_with_gimple_body (struct cgraph_node *node)
>    return NULL;
>  }
>  
> +/* Return true iff NODE is the main function (main in C).  */
> +static inline bool
> +cgraph_main_function_p (struct cgraph_node *node)
> +{
> +  return node->local.main_function;

node && node->local.main_function

> +}
> +
>  /* Walk all functions with body defined.  */
>  #define FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
>     for ((node) = cgraph_first_function_with_gimple_body (); (node); \
> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> index 516f187..4a59f63 100644
> --- a/gcc/cgraphunit.c
> +++ b/gcc/cgraphunit.c
> @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested)
>    notice_global_symbol (decl);
>    node->local.finalized = true;
>    node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
> +  node->local.main_function =
> +    DECL_FILE_SCOPE_P (decl)
> +    && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl)))
> +	|| decl_assembler_name_equal (decl, main_identifier_node));

If we finalize a function we should always create an assembler name,
thus I'd change the above to

  node->local.main_function = decl_assembler_name_equal (decl, 
main_identifier_node);

btw, decl_assembler_name_equal doesn't seem to remove target-specific
mangling - do some OSes "mangle" main differently (I'm thinking of
leading underscores or complete renames)?  Thus, I guess the
targets might want to be able to provide the main_identifier_assember_name
you use here.

>    if (cgraph_decide_is_function_needed (node, decl))
>      cgraph_mark_needed_node (node);
> diff --git a/gcc/config/i386/cygming.h b/gcc/config/i386/cygming.h
> index 2eccda9..8ac169e 100644
> --- a/gcc/config/i386/cygming.h
> +++ b/gcc/config/i386/cygming.h
> @@ -360,7 +360,8 @@ do {						\
>  
>  #undef PROFILE_HOOK
>  #define PROFILE_HOOK(LABEL)						\
> -  if (MAIN_NAME_P (DECL_NAME (current_function_decl)))			\
> +  if (DECL_FILE_SCOPE_P (current_function_decl)				\
> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl))) \
>      {

See above.
									\
>        emit_call_insn (gen_rtx_CALL (VOIDmode,				\
>  	gen_rtx_MEM (FUNCTION_MODE,					\
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 78a366e..5a3a712 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -9509,9 +9509,8 @@ ix86_minimum_incoming_stack_boundary (bool sibcall)
>    /* Stack at entrance of main is aligned by runtime.  We use the
>       smallest incoming stack boundary. */
>    if (incoming_stack_boundary > MAIN_STACK_BOUNDARY
> -      && DECL_NAME (current_function_decl)
> -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
> -      && DECL_FILE_SCOPE_P (current_function_decl))
> +      && DECL_FILE_SCOPE_P (current_function_decl)
> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
>      incoming_stack_boundary = MAIN_STACK_BOUNDARY;

Likewise.

>    return incoming_stack_boundary;
> diff --git a/gcc/config/pdp11/pdp11.c b/gcc/config/pdp11/pdp11.c
> index 42e3af0..aa38903 100644
> --- a/gcc/config/pdp11/pdp11.c
> +++ b/gcc/config/pdp11/pdp11.c
> @@ -240,7 +240,9 @@ pdp11_expand_prologue (void)
>  
>    /* If we are outputting code for main, the switch FPU to the
>       right mode if TARGET_FPU.  */
> -  if (MAIN_NAME_P (DECL_NAME (current_function_decl)) && TARGET_FPU)
> +  if (TARGET_FPU
> +      && DECL_FILE_SCOPE_P (current_function_decl)
> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
>      {

Likewise.

The rest looks good to me.  Honza?

Thanks,
Richard.

>        emit_insn (gen_setd ());
>        emit_insn (gen_seti ());
> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
> index 8a1dd2e..6a1a494 100644
> --- a/gcc/fortran/trans-decl.c
> +++ b/gcc/fortran/trans-decl.c
> @@ -4922,7 +4922,6 @@ create_main_function (tree fndecl)
>    tmp =  build_function_type_list (integer_type_node, integer_type_node,
>  				   build_pointer_type (pchar_type_node),
>  				   NULL_TREE);
> -  main_identifier_node = get_identifier ("main");
>    ftn_main = build_decl (input_location, FUNCTION_DECL,
>        			 main_identifier_node, tmp);
>    DECL_EXTERNAL (ftn_main) = 0;
> diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c
> index 9dddf39..595330e 100644
> --- a/gcc/ipa-split.c
> +++ b/gcc/ipa-split.c
> @@ -1409,7 +1409,7 @@ execute_split_functions (void)
>  	fprintf (dump_file, "Not splitting: noreturn/malloc function.\n");
>        return 0;
>      }
> -  if (MAIN_NAME_P (DECL_NAME (current_function_decl)))
> +  if (cgraph_main_function_p (node))
>      {
>        if (dump_file)
>  	fprintf (dump_file, "Not splitting: main function.\n");
> diff --git a/gcc/ipa.c b/gcc/ipa.c
> index 388291a..f9dc42d 100644
> --- a/gcc/ipa.c
> +++ b/gcc/ipa.c
> @@ -639,7 +639,7 @@ cgraph_externally_visible_p (struct cgraph_node *node,
>    else if (!whole_program)
>      return true;
>  
> -  if (MAIN_NAME_P (DECL_NAME (node->decl)))
> +  if (cgraph_main_function_p (node))
>      return true;
>  
>    return false;
> diff --git a/gcc/lto-cgraph.c b/gcc/lto-cgraph.c
> index 5e899bc..34bcc55 100644
> --- a/gcc/lto-cgraph.c
> +++ b/gcc/lto-cgraph.c
> @@ -502,6 +502,7 @@ lto_output_node (struct lto_simple_output_block *ob, struct cgraph_node *node,
>    bp_pack_value (&bp, node->local.versionable, 1);
>    bp_pack_value (&bp, node->local.can_change_signature, 1);
>    bp_pack_value (&bp, node->local.redefined_extern_inline, 1);
> +  bp_pack_value (&bp, node->local.main_function, 1);
>    bp_pack_value (&bp, node->needed, 1);
>    bp_pack_value (&bp, node->address_taken, 1);
>    bp_pack_value (&bp, node->abstract_and_needed, 1);
> @@ -904,6 +905,7 @@ input_overwrite_node (struct lto_file_decl_data *file_data,
>    node->local.versionable = bp_unpack_value (bp, 1);
>    node->local.can_change_signature = bp_unpack_value (bp, 1);
>    node->local.redefined_extern_inline = bp_unpack_value (bp, 1);
> +  node->local.main_function = bp_unpack_value (bp, 1);
>    node->needed = bp_unpack_value (bp, 1);
>    node->address_taken = bp_unpack_value (bp, 1);
>    node->abstract_and_needed = bp_unpack_value (bp, 1);
> diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c
> index 999db8b..a44a35f 100644
> --- a/gcc/lto/lto-lang.c
> +++ b/gcc/lto/lto-lang.c
> @@ -1137,13 +1137,6 @@ lto_init (void)
>    /* Create the basic integer types.  */
>    build_common_tree_nodes (flag_signed_char, /*short_double=*/false);
>  
> -  /* The global tree for the main identifier is filled in by
> -     language-specific front-end initialization that is not run in the
> -     LTO back-end.  It appears that all languages that perform such
> -     initialization currently do so in the same way, so we do it here.  */
> -  if (main_identifier_node == NULL_TREE)
> -    main_identifier_node = get_identifier ("main");
> -
>    /* In the C++ front-end, fileptr_type_node is defined as a variant
>       copy of of ptr_type_node, rather than ptr_node itself.  The
>       distinction should only be relevant to the front-end, so we
> diff --git a/gcc/predict.c b/gcc/predict.c
> index c12b45f..819e64c 100644
> --- a/gcc/predict.c
> +++ b/gcc/predict.c
> @@ -2275,7 +2275,7 @@ compute_function_frequency (void)
>    basic_block bb;
>    struct cgraph_node *node = cgraph_get_node (current_function_decl);
>    if (DECL_STATIC_CONSTRUCTOR (current_function_decl)
> -      || MAIN_NAME_P (DECL_NAME (current_function_decl)))
> +      || cgraph_main_function_p (node))
>      node->only_called_at_startup = true;
>    if (DECL_STATIC_DESTRUCTOR (current_function_decl))
>      node->only_called_at_exit = true;
> @@ -2291,7 +2291,7 @@ compute_function_frequency (void)
>          node->frequency = NODE_FREQUENCY_HOT;
>        else if (flags & ECF_NORETURN)
>          node->frequency = NODE_FREQUENCY_EXECUTED_ONCE;
> -      else if (MAIN_NAME_P (DECL_NAME (current_function_decl)))
> +      else if (cgraph_main_function_p (node))
>          node->frequency = NODE_FREQUENCY_EXECUTED_ONCE;
>        else if (DECL_STATIC_CONSTRUCTOR (current_function_decl)
>  	       || DECL_STATIC_DESTRUCTOR (current_function_decl))
> diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
> index b65f5aa..9cb7a16 100644
> --- a/gcc/tree-ssa-structalias.c
> +++ b/gcc/tree-ssa-structalias.c
> @@ -6933,7 +6933,7 @@ ipa_pta_execute (void)
>  
>  	  /* We also need to make function return values escape.  Nothing
>  	     escapes by returning from main though.  */
> -	  if (!MAIN_NAME_P (DECL_NAME (node->decl)))
> +	  if (!cgraph_main_function_p (node->decl))
>  	    {
>  	      varinfo_t fi, rvi;
>  	      fi = lookup_vi_for_tree (node->decl);
> diff --git a/gcc/tree.c b/gcc/tree.c
> index cfea9f7..d3bbeb0 100644
> --- a/gcc/tree.c
> +++ b/gcc/tree.c
> @@ -9477,6 +9477,8 @@ build_common_tree_nodes (bool signed_char, bool short_double)
>  
>      va_list_type_node = t;
>    }
> +
> +  main_identifier_node = get_identifier ("main");
>  }
>  
>  /* A subroutine of build_common_builtin_nodes.  Define a builtin function.  */
> 
>

On Mar 20, 2012, at 1:21 PM, Richard Guenther wrote:

> On Tue, 20 Mar 2012, Tristan Gingold wrote:
> 
>> 
>> On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:
>> 
>>> On Wed, 14 Mar 2012, Tristan Gingold wrote:
>> […]
>> 
>>> 
>>> Well.  To make this work in LTO the "main" function (thus, the program
>>> entry point) should be marked at cgraph level and all users of
>>> MAIN_NAME_P should instead check a flag on the cgraph node.
>>> 
>>>> Will write a predicate in tree.[ch].
>>> 
>>> Please instead transition "main-ness" to the graph.
>> 
>> Hi,
>> 
>> here is the patch I wrote.  Does it match what you had in mind ?
> 
> Basically yes.  Comments below.
> 
>> main_identifier_node is now set in tree.c
> 
> Looks good, hopefully my review-grep was as good as yours ;)

[…]

>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
>> index bd21169..7a7a774 100644
>> --- a/gcc/cfgexpand.c
>> +++ b/gcc/cfgexpand.c
>> @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void)
>> 
>>   /* If this function is `main', emit a call to `__main'
>>      to run global initializers, etc.  */
>> -  if (DECL_NAME (current_function_decl)
>> -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
>> -      && DECL_FILE_SCOPE_P (current_function_decl))
>> +  if (DECL_FILE_SCOPE_P (current_function_decl)
>> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
>>     expand_main_function ();
> 
> The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere
> you call cgraph_main_function_p.  I suppose returning false if the
> cgraph node is NULL in cgraph_main_function_p would be good.

Ok.  (I added the DECL_FILE_SCOPE_P check to avoid the cgraph lookup for speed reason)

[…]

>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>> index 516f187..4a59f63 100644
>> --- a/gcc/cgraphunit.c
>> +++ b/gcc/cgraphunit.c
>> @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested)
>>   notice_global_symbol (decl);
>>   node->local.finalized = true;
>>   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
>> +  node->local.main_function =
>> +    DECL_FILE_SCOPE_P (decl)
>> +    && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl)))
>> +	|| decl_assembler_name_equal (decl, main_identifier_node));
> 
> If we finalize a function we should always create an assembler name,
> thus I'd change the above to
> 
>  node->local.main_function = decl_assembler_name_equal (decl, 
> main_identifier_node);

Indeed.  At worst, the assembler name is created during the call to notice_global_symbol.

> btw, decl_assembler_name_equal doesn't seem to remove target-specific
> mangling - do some OSes "mangle" main differently (I'm thinking of
> leading underscores or complete renames)?  Thus, I guess the
> targets might want to be able to provide the main_identifier_assember_name
> you use here.

I think this is currently OK because decl_assembler_name_equal deals
with leading underscore correctly.  I have checked that on Darwin,
which has a leading underscore.

The only target that mangle names is i386 cygwin/mingw, which 'annotates'
stdcall and fastcall function, but main() is regular.

But I agree this mechanism is fragile.

In order to make this mechanism stronger, we could add main_function_node, which
designates the FUNCTION_DECL that is the main function (if not NULL_TREE), with
a fallback on main_identifier_node for regular languages such as C or C++.

Tristan.

On Tue, 20 Mar 2012, Tristan Gingold wrote:

> 
> On Mar 20, 2012, at 1:21 PM, Richard Guenther wrote:
> 
> > On Tue, 20 Mar 2012, Tristan Gingold wrote:
> > 
> >> 
> >> On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:
> >> 
> >>> On Wed, 14 Mar 2012, Tristan Gingold wrote:
> >> […]
> >> 
> >>> 
> >>> Well.  To make this work in LTO the "main" function (thus, the program
> >>> entry point) should be marked at cgraph level and all users of
> >>> MAIN_NAME_P should instead check a flag on the cgraph node.
> >>> 
> >>>> Will write a predicate in tree.[ch].
> >>> 
> >>> Please instead transition "main-ness" to the graph.
> >> 
> >> Hi,
> >> 
> >> here is the patch I wrote.  Does it match what you had in mind ?
> > 
> > Basically yes.  Comments below.
> > 
> >> main_identifier_node is now set in tree.c
> > 
> > Looks good, hopefully my review-grep was as good as yours ;)
> 
> […]
> 
> >> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> >> index bd21169..7a7a774 100644
> >> --- a/gcc/cfgexpand.c
> >> +++ b/gcc/cfgexpand.c
> >> @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void)
> >> 
> >>   /* If this function is `main', emit a call to `__main'
> >>      to run global initializers, etc.  */
> >> -  if (DECL_NAME (current_function_decl)
> >> -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
> >> -      && DECL_FILE_SCOPE_P (current_function_decl))
> >> +  if (DECL_FILE_SCOPE_P (current_function_decl)
> >> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
> >>     expand_main_function ();
> > 
> > The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere
> > you call cgraph_main_function_p.  I suppose returning false if the
> > cgraph node is NULL in cgraph_main_function_p would be good.
> 
> Ok.  (I added the DECL_FILE_SCOPE_P check to avoid the cgraph lookup for speed reason)
> 
> […]
> 
> >> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> >> index 516f187..4a59f63 100644
> >> --- a/gcc/cgraphunit.c
> >> +++ b/gcc/cgraphunit.c
> >> @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested)
> >>   notice_global_symbol (decl);
> >>   node->local.finalized = true;
> >>   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
> >> +  node->local.main_function =
> >> +    DECL_FILE_SCOPE_P (decl)
> >> +    && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl)))
> >> +	|| decl_assembler_name_equal (decl, main_identifier_node));
> > 
> > If we finalize a function we should always create an assembler name,
> > thus I'd change the above to
> > 
> >  node->local.main_function = decl_assembler_name_equal (decl, 
> > main_identifier_node);
> 
> Indeed.  At worst, the assembler name is created during the call to notice_global_symbol.
> 
> > btw, decl_assembler_name_equal doesn't seem to remove target-specific
> > mangling - do some OSes "mangle" main differently (I'm thinking of
> > leading underscores or complete renames)?  Thus, I guess the
> > targets might want to be able to provide the main_identifier_assember_name
> > you use here.
> 
> I think this is currently OK because decl_assembler_name_equal deals
> with leading underscore correctly.  I have checked that on Darwin,
> which has a leading underscore.
> 
> The only target that mangle names is i386 cygwin/mingw, which 'annotates'
> stdcall and fastcall function, but main() is regular.
> 
> But I agree this mechanism is fragile.
> 
> In order to make this mechanism stronger, we could add main_function_node, which
> designates the FUNCTION_DECL that is the main function (if not NULL_TREE), with
> a fallback on main_identifier_node for regular languages such as C or C++.

I'd rather get away from using a global main_identifier_node, instead
make that frontend specific, and introduce targetm.main_assembler_name
which the assembler-name creating langhook would make sure to use
when mangling what the FE thinks main is.  main_identifier_node should
not serve any purpose outside of Frontends.

But I see both as a possible cleanup opportunity, not a necessary change.

Richard.

> On Tue, 20 Mar 2012, Tristan Gingold wrote:
> 
> > 
> > On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:
> > 
> > > On Wed, 14 Mar 2012, Tristan Gingold wrote:
> > [?]
> > 
> > > 
> > > Well.  To make this work in LTO the "main" function (thus, the program
> > > entry point) should be marked at cgraph level and all users of
> > > MAIN_NAME_P should instead check a flag on the cgraph node.
> > > 
> > >> Will write a predicate in tree.[ch].
> > > 
> > > Please instead transition "main-ness" to the graph.

Yep, I also agree that it is something cgraph code should care about instead of
random placess across the whole middle-end.
> > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> > index bd21169..7a7a774 100644
> > --- a/gcc/cfgexpand.c
> > +++ b/gcc/cfgexpand.c
> > @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void)
> >  
> >    /* If this function is `main', emit a call to `__main'
> >       to run global initializers, etc.  */
> > -  if (DECL_NAME (current_function_decl)
> > -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
> > -      && DECL_FILE_SCOPE_P (current_function_decl))
> > +  if (DECL_FILE_SCOPE_P (current_function_decl)
> > +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
> >      expand_main_function ();
> 
> The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere
> you call cgraph_main_function_p.  I suppose returning false if the
> cgraph node is NULL in cgraph_main_function_p would be good.

How do we handle the cases before cgraph is built with this approach?
> > +/* Return true iff NODE is the main function (main in C).  */
> > +static inline bool
> > +cgraph_main_function_p (struct cgraph_node *node)
> > +{
> > +  return node->local.main_function;
> 
> node && node->local.main_function

Well, cgraph strategy is ito ICE when NODE is NULL :)
We could have cgraph_main_function_decl_p wrapper that does the NULL handling, but I still don't
see how this helps - i.e. when you don't have cgraph node you don't have info whether function
is main or not, so you should not even try to ask.
In what cases we ICE here?
> 
> > +}
> > +
> >  /* Walk all functions with body defined.  */
> >  #define FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
> >     for ((node) = cgraph_first_function_with_gimple_body (); (node); \
> > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
> > index 516f187..4a59f63 100644
> > --- a/gcc/cgraphunit.c
> > +++ b/gcc/cgraphunit.c
> > @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested)
> >    notice_global_symbol (decl);
> >    node->local.finalized = true;
> >    node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
> > +  node->local.main_function =
> > +    DECL_FILE_SCOPE_P (decl)
> > +    && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl)))
> > +	|| decl_assembler_name_equal (decl, main_identifier_node));
> 
> If we finalize a function we should always create an assembler name,
> thus I'd change the above to
> 
>   node->local.main_function = decl_assembler_name_equal (decl, 
> main_identifier_node);
> 
> btw, decl_assembler_name_equal doesn't seem to remove target-specific
> mangling - do some OSes "mangle" main differently (I'm thinking of
> leading underscores or complete renames)?  Thus, I guess the
> targets might want to be able to provide the main_identifier_assember_name
> you use here.

Yes, name function is mangled, i.e. it is _main on djgpp as long as I remember.
This is why we have the main_identifier_node to go through the mandling procedure.

Honza

On Mar 20, 2012, at 6:17 PM, Jan Hubicka wrote:

>> On Tue, 20 Mar 2012, Tristan Gingold wrote:
>> 
>>> 
>>> On Mar 15, 2012, at 10:37 AM, Richard Guenther wrote:
>>> 
>>>> On Wed, 14 Mar 2012, Tristan Gingold wrote:
>>> [?]
>>> 
>>>> 
>>>> Well.  To make this work in LTO the "main" function (thus, the program
>>>> entry point) should be marked at cgraph level and all users of
>>>> MAIN_NAME_P should instead check a flag on the cgraph node.
>>>> 
>>>>> Will write a predicate in tree.[ch].
>>>> 
>>>> Please instead transition "main-ness" to the graph.
> 
> Yep, I also agree that it is something cgraph code should care about instead of
> random placess across the whole middle-end.
>>> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
>>> index bd21169..7a7a774 100644
>>> --- a/gcc/cfgexpand.c
>>> +++ b/gcc/cfgexpand.c
>>> @@ -4513,9 +4513,8 @@ gimple_expand_cfg (void)
>>> 
>>>   /* If this function is `main', emit a call to `__main'
>>>      to run global initializers, etc.  */
>>> -  if (DECL_NAME (current_function_decl)
>>> -      && MAIN_NAME_P (DECL_NAME (current_function_decl))
>>> -      && DECL_FILE_SCOPE_P (current_function_decl))
>>> +  if (DECL_FILE_SCOPE_P (current_function_decl)
>>> +      && cgraph_main_function_p (cgraph_get_node (current_function_decl)))
>>>     expand_main_function ();
>> 
>> The DECL_FILE_SCOPE_P check is redundant, please remove them everywhere
>> you call cgraph_main_function_p.  I suppose returning false if the
>> cgraph node is NULL in cgraph_main_function_p would be good.
> 
> How do we handle the cases before cgraph is built with this approach?

Only front-end code need to check wether a function is main before they add
it in cgraph.  As each front-end should know which function is main, this is
not an issue for them.

>>> +/* Return true iff NODE is the main function (main in C).  */
>>> +static inline bool
>>> +cgraph_main_function_p (struct cgraph_node *node)
>>> +{
>>> +  return node->local.main_function;
>> 
>> node && node->local.main_function
> 
> Well, cgraph strategy is ito ICE when NODE is NULL :)
> We could have cgraph_main_function_decl_p wrapper that does the NULL handling, but I still don't
> see how this helps - i.e. when you don't have cgraph node you don't have info whether function
> is main or not, so you should not even try to ask.
> In what cases we ICE here?

We don't ICE here - as long as graph_main_function_p is called after front-end.

>>> +}
>>> +
>>> /* Walk all functions with body defined.  */
>>> #define FOR_EACH_FUNCTION_WITH_GIMPLE_BODY(node) \
>>>    for ((node) = cgraph_first_function_with_gimple_body (); (node); \
>>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
>>> index 516f187..4a59f63 100644
>>> --- a/gcc/cgraphunit.c
>>> +++ b/gcc/cgraphunit.c
>>> @@ -346,6 +346,10 @@ cgraph_finalize_function (tree decl, bool nested)
>>>   notice_global_symbol (decl);
>>>   node->local.finalized = true;
>>>   node->lowered = DECL_STRUCT_FUNCTION (decl)->cfg != NULL;
>>> +  node->local.main_function =
>>> +    DECL_FILE_SCOPE_P (decl)
>>> +    && ((!DECL_ASSEMBLER_NAME_SET_P (decl) && MAIN_NAME_P (DECL_NAME (decl)))
>>> +	||decl_assembler_name_equal (decl, main_identifier_node));
>> 
>> If we finalize a function we should always create an assembler name,
>> thus I'd change the above to
>> 
>>  node->local.main_function = decl_assembler_name_equal (decl, 
>> main_identifier_node);
>> 
>> btw, decl_assembler_name_equal doesn't seem to remove target-specific
>> mangling - do some OSes "mangle" main differently (I'm thinking of
>> leading underscores or complete renames)?  Thus, I guess the
>> targets might want to be able to provide the main_identifier_assember_name
>> you use here.
> 
> Yes, name function is mangled, i.e. it is _main on djgpp as long as I remember.
> This is why we have the main_identifier_node to go through the mandling procedure.


USER_LABEL_PREFIX is handled by decl_assembler_name_equal.

One way to simplify that is to change the NESTED argument of cgraph_finalize_function
to LEVEL, which could be either main, top or nested.  With this mechanism, every
front-end will explicitly tell to the middle-end which function is the main entry point.

Thoughts ?

Tristan.

whic