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

2012-03-21 Thread Richard Guenther
On Tue, 20 Mar 2012, Tristan Gingold wrote:

 
 On Mar 20, 2012, at 5:01 PM, Richard Guenther wrote:
 
  On Tue, 20 Mar 2012, Tristan Gingold wrote:
  
  
  On Mar 20, 2012, at 3:19 PM, Richard Guenther wrote:
  
  [...]
  
  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.
  
  Something along these lines ?
  
  Yes, but I'd simply call the hook at the places you now use
  main_assembler_name and not create a global tree node for it.
 
 But we use it at the beginning of graph_finalize_function, so caching it
 makes sense, doesn't it ?

Well, maybe ;)  I have no strong opinion here.

Richard.


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

2012-03-21 Thread Tristan Gingold

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


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

2012-03-20 Thread Tristan Gingold

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.

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 ();
 
   /* 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;
+}
+
 /* 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 (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 

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

2012-03-20 Thread Richard Guenther
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,

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

2012-03-20 Thread Tristan Gingold

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.



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

2012-03-20 Thread Richard Guenther
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.

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

2012-03-20 Thread Tristan Gingold

On Mar 20, 2012, at 3:19 PM, Richard Guenther wrote:

[…]
 
 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.

Something along these lines ?

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/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
index 7383358..b0fa085d 100644
--- a/gcc/ada/gcc-interface/utils.c
+++ b/gcc/ada/gcc-interface/utils.c
@@ -1902,14 +1902,12 @@ create_subprog_decl (tree subprog_name, tree asm_name, 
tree subprog_type,
 {
   SET_DECL_ASSEMBLER_NAME (subprog_decl, asm_name);
 
-  /* The expand_main_function circuitry expects main_identifier_node to
-designate the DECL_NAME of the 'main' entry point, in turn expected
-to be declared as the main function literally by default.  Ada
-program entry points are typically declared with a different name
+  /* Ada program entry points are typically declared with a different name
 within the binder generated file, exported as 'main' to satisfy the
-system expectations.  Force main_identifier_node in this case.  */
-  if (asm_name == main_identifier_node)
-   DECL_NAME (subprog_decl) = main_identifier_node;
+system expectations.  Force main_assembler_node in this case.  */
+  if (IDENTIFIER_LENGTH (asm_name) == 4
+  memcmp (IDENTIFIER_POINTER (asm_name), main, 4) == 0)
+   DECL_NAME (subprog_decl) = main_assembler_name;
 }
 
   /* Add this decl to the current binding level.  */
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 835b13b..fea5181 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -291,6 +291,8 @@ enum c_tree_index
 
 CTI_DEFAULT_FUNCTION_TYPE,
 
+CTI_MAIN_IDENTIFIER,
+
 /* These are not types, but we have to look them up all the time.  */
 CTI_FUNCTION_NAME_DECL,
 CTI_PRETTY_FUNCTION_NAME_DECL,
@@ -426,6 +428,10 @@ extern const unsigned int num_c_common_reswords;
 
 #define default_function_type  
c_global_trees[CTI_DEFAULT_FUNCTION_TYPE]
 
+#define main_identifier_node   c_global_trees[CTI_MAIN_IDENTIFIER]
+#define MAIN_NAME_P(NODE) \
+  (IDENTIFIER_NODE_CHECK (NODE) == main_identifier_node)
+
 #define function_name_decl_node
c_global_trees[CTI_FUNCTION_NAME_DECL]
 #define pretty_function_name_decl_node 
c_global_trees[CTI_PRETTY_FUNCTION_NAME_DECL]
 #define c99_function_name_decl_node
c_global_trees[CTI_C99_FUNCTION_NAME_DECL]
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index bd21169..db53309 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -4513,9 +4513,7 @@ 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 (cgraph_main_function_p (cgraph_get_node (current_function_decl)))
 expand_main_function ();
 
   /* 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..089d851 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;
 };
 
 /* 

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

2012-03-20 Thread Richard Guenther
On Tue, 20 Mar 2012, Tristan Gingold wrote:

 
 On Mar 20, 2012, at 3:19 PM, Richard Guenther wrote:
 
 […]
  
  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.
 
 Something along these lines ?

Yes, but I'd simply call the hook at the places you now use
main_assembler_name and not create a global tree node for it.

Richard.

 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/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
 index 7383358..b0fa085d 100644
 --- a/gcc/ada/gcc-interface/utils.c
 +++ b/gcc/ada/gcc-interface/utils.c
 @@ -1902,14 +1902,12 @@ create_subprog_decl (tree subprog_name, tree 
 asm_name, tree subprog_type,
  {
SET_DECL_ASSEMBLER_NAME (subprog_decl, asm_name);
  
 -  /* The expand_main_function circuitry expects main_identifier_node to
 -  designate the DECL_NAME of the 'main' entry point, in turn expected
 -  to be declared as the main function literally by default.  Ada
 -  program entry points are typically declared with a different name
 +  /* Ada program entry points are typically declared with a different 
 name
within the binder generated file, exported as 'main' to satisfy the
 -  system expectations.  Force main_identifier_node in this case.  */
 -  if (asm_name == main_identifier_node)
 - DECL_NAME (subprog_decl) = main_identifier_node;
 +  system expectations.  Force main_assembler_node in this case.  */
 +  if (IDENTIFIER_LENGTH (asm_name) == 4
 +memcmp (IDENTIFIER_POINTER (asm_name), main, 4) == 0)
 + DECL_NAME (subprog_decl) = main_assembler_name;
  }
  
/* Add this decl to the current binding level.  */
 diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
 index 835b13b..fea5181 100644
 --- a/gcc/c-family/c-common.h
 +++ b/gcc/c-family/c-common.h
 @@ -291,6 +291,8 @@ enum c_tree_index
  
  CTI_DEFAULT_FUNCTION_TYPE,
  
 +CTI_MAIN_IDENTIFIER,
 +
  /* These are not types, but we have to look them up all the time.  */
  CTI_FUNCTION_NAME_DECL,
  CTI_PRETTY_FUNCTION_NAME_DECL,
 @@ -426,6 +428,10 @@ extern const unsigned int num_c_common_reswords;
  
  #define default_function_type
 c_global_trees[CTI_DEFAULT_FUNCTION_TYPE]
  
 +#define main_identifier_node c_global_trees[CTI_MAIN_IDENTIFIER]
 +#define MAIN_NAME_P(NODE) \
 +  (IDENTIFIER_NODE_CHECK (NODE) == main_identifier_node)
 +
  #define function_name_decl_node  
 c_global_trees[CTI_FUNCTION_NAME_DECL]
  #define pretty_function_name_decl_node   
 c_global_trees[CTI_PRETTY_FUNCTION_NAME_DECL]
  #define c99_function_name_decl_node  
 c_global_trees[CTI_C99_FUNCTION_NAME_DECL]
 diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
 index bd21169..db53309 100644
 --- a/gcc/cfgexpand.c
 +++ b/gcc/cfgexpand.c
 @@ -4513,9 +4513,7 @@ 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 (cgraph_main_function_p (cgraph_get_node (current_function_decl)))
  expand_main_function ();
  
/* 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..089d851 100644
 --- a/gcc/cgraph.h
 +++ b/gcc/cgraph.h
 @@ 

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

2012-03-20 Thread Tristan Gingold

On Mar 20, 2012, at 5:01 PM, Richard Guenther wrote:

 On Tue, 20 Mar 2012, Tristan Gingold wrote:
 
 
 On Mar 20, 2012, at 3:19 PM, Richard Guenther wrote:
 
 […]
 
 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.
 
 Something along these lines ?
 
 Yes, but I'd simply call the hook at the places you now use
 main_assembler_name and not create a global tree node for it.

But we use it at the beginning of graph_finalize_function, so caching it
makes sense, doesn't it ?

Tristan.



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

2012-03-15 Thread Richard Guenther
On Wed, 14 Mar 2012, Tristan Gingold wrote:

 
 On Mar 14, 2012, at 5:08 PM, Richard Guenther wrote:
 
  On Wed, 14 Mar 2012, Tristan Gingold wrote:
  
  Hi,
  
  the code to call expand_main_function currently only checks DECL_NAME.  
  This leads
  to a hack in ada/gcc-interface/utils.c to handle the gnatbind generated 
  file that could
  declare:
  
  package ada_main is
  …
function my_main
  (argc : Integer;
   argv : System.Address;
   envp : System.Address)
   return Integer;
pragma Export (C, my_main, main);
  …
  end ada_main;
  
  But expand_main_function is also called for function whose name is main 
  but assembly name isn't.  Eg:
  
  package pkg is
procedure main;
  end pkg;
  
  So I think we should consider the assembler name is set, otherwise the 
  decl name.
  
  Manually tested on ia64-hp-openvms (where this issue was discovered).
  No C regressions for x86_64-darwin.
  
  Ok for trunk ?
  
  There are more checks for MAIN_NAME_P, so this certainly isn't enough.
  And if it is a good idea then the whole check, whether a FUNCTION_DECL
  is considered 'main' should be put into a function in tree.[ch] and
  used everywhere.  Note that what is 'main' is controlled by
  main_identifier_node, controlled by frontends.  So - why is that not
  enough to control for Ada?
 
 Indeed, I think we could handle this issue in gigi for Ada.  (I also think
 we don't want to handle crazy C code such as 'int my_main () asm (main)'.
 
 But, unless I missed something, doing this in gigi won't work with LTO.

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 cgraph.

Thanks,
Richard.

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

2012-03-15 Thread Tristan Gingold

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

 On Wed, 14 Mar 2012, Tristan Gingold wrote:
 
 
 On Mar 14, 2012, at 5:08 PM, Richard Guenther wrote:
 
 On Wed, 14 Mar 2012, Tristan Gingold wrote:
 
 Hi,
 
 the code to call expand_main_function currently only checks DECL_NAME.  
 This leads
 to a hack in ada/gcc-interface/utils.c to handle the gnatbind generated 
 file that could
 declare:
 
 package ada_main is
 …
  function my_main
(argc : Integer;
 argv : System.Address;
 envp : System.Address)
 return Integer;
  pragma Export (C, my_main, main);
 …
 end ada_main;
 
 But expand_main_function is also called for function whose name is main 
 but assembly name isn't.  Eg:
 
 package pkg is
  procedure main;
 end pkg;
 
 So I think we should consider the assembler name is set, otherwise the 
 decl name.
 
 Manually tested on ia64-hp-openvms (where this issue was discovered).
 No C regressions for x86_64-darwin.
 
 Ok for trunk ?
 
 There are more checks for MAIN_NAME_P, so this certainly isn't enough.
 And if it is a good idea then the whole check, whether a FUNCTION_DECL
 is considered 'main' should be put into a function in tree.[ch] and
 used everywhere.  Note that what is 'main' is controlled by
 main_identifier_node, controlled by frontends.  So - why is that not
 enough to control for Ada?
 
 Indeed, I think we could handle this issue in gigi for Ada.  (I also think
 we don't want to handle crazy C code such as 'int my_main () asm (main)'.
 
 But, unless I missed something, doing this in gigi won't work with LTO.
 
 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.

Ok, I will explore this way.

Tristan.



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

2012-03-14 Thread Richard Guenther
On Wed, 14 Mar 2012, Tristan Gingold wrote:

 Hi,
 
 the code to call expand_main_function currently only checks DECL_NAME.  This 
 leads
 to a hack in ada/gcc-interface/utils.c to handle the gnatbind generated file 
 that could
 declare:
 
 package ada_main is
 …
function my_main
  (argc : Integer;
   argv : System.Address;
   envp : System.Address)
   return Integer;
pragma Export (C, my_main, main);
 …
 end ada_main;
 
 But expand_main_function is also called for function whose name is main but 
 assembly name isn't.  Eg:
 
 package pkg is
procedure main;
 end pkg;
 
 So I think we should consider the assembler name is set, otherwise the decl 
 name.
 
 Manually tested on ia64-hp-openvms (where this issue was discovered).
 No C regressions for x86_64-darwin.
 
 Ok for trunk ?

There are more checks for MAIN_NAME_P, so this certainly isn't enough.
And if it is a good idea then the whole check, whether a FUNCTION_DECL
is considered 'main' should be put into a function in tree.[ch] and
used everywhere.  Note that what is 'main' is controlled by
main_identifier_node, controlled by frontends.  So - why is that not
enough to control for Ada?

Richard.

 Tristan.
 
 gcc/
 2012-03-14  Tristan Gingold  ging...@adacore.com
 
   * cfgexpand.c (gimple_expand_cfg): Consider the assembly name
   to call expand_main_function.
 
 gcc/ada/
 2012-03-14  Tristan Gingold  ging...@adacore.com
 
   * gcc-interface/utils.c (create_subprog_decl): Do not override
   DECL_NAME if asm_name is set.
 
 
 diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
 index 2f38bb4..8693876 100644
 --- a/gcc/cfgexpand.c
 +++ b/gcc/cfgexpand.c
 @@ -4501,8 +4501,12 @@ 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))
 +   DECL_FILE_SCOPE_P (current_function_decl)
 +   main_identifier_node != NULL_TREE
 +   ((!DECL_ASSEMBLER_NAME_SET_P (current_function_decl)
 + MAIN_NAME_P (DECL_NAME (current_function_decl)))
 +   || decl_assembler_name_equal (current_function_decl,
 + main_identifier_node)))
  expand_main_function ();
  
/* Initialize the stack_protect_guard field.  This must happen after the
 
 diff --git a/gcc/ada/gcc-interface/utils.c b/gcc/ada/gcc-interface/utils.c
 index 7383358..81a1a0a 100644
 --- a/gcc/ada/gcc-interface/utils.c
 +++ b/gcc/ada/gcc-interface/utils.c
 @@ -1899,18 +1899,7 @@ create_subprog_decl (tree subprog_name, tree asm_name, 
 tree subprog_type,
DECL_RESULT (subprog_decl) = result_decl;
  
if (asm_name)
 -{
 -  SET_DECL_ASSEMBLER_NAME (subprog_decl, asm_name);
 -
 -  /* The expand_main_function circuitry expects main_identifier_node to
 -  designate the DECL_NAME of the 'main' entry point, in turn expected
 -  to be declared as the main function literally by default.  Ada
 -  program entry points are typically declared with a different name
 -  within the binder generated file, exported as 'main' to satisfy the
 -  system expectations.  Force main_identifier_node in this case.  */
 -  if (asm_name == main_identifier_node)
 - DECL_NAME (subprog_decl) = main_identifier_node;
 -}
 +SET_DECL_ASSEMBLER_NAME (subprog_decl, asm_name);
  
/* Add this decl to the current binding level.  */
gnat_pushdecl (subprog_decl, gnat_node);
 
 
 

-- 
Richard Guenther rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer