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

login
register
mail settings
Submitter Tristan Gingold
Date March 14, 2012, 4:04 p.m.
Message ID <875C6F56-7161-48DF-91FB-3A01891F8289@adacore.com>
Download mbox | patch
Permalink /patch/146664/
State New
Headers show

Comments

Tristan Gingold - March 14, 2012, 4:04 p.m.
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 ?

Tristan.

gcc/
2012-03-14  Tristan Gingold  <gingold@adacore.com>

	* cfgexpand.c (gimple_expand_cfg): Consider the assembly name
	to call expand_main_function.

gcc/ada/
2012-03-14  Tristan Gingold  <gingold@adacore.com>

	* gcc-interface/utils.c (create_subprog_decl): Do not override
	DECL_NAME if asm_name is set.
Richard Guenther - March 14, 2012, 4:08 p.m.
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  <gingold@adacore.com>
> 
> 	* cfgexpand.c (gimple_expand_cfg): Consider the assembly name
> 	to call expand_main_function.
> 
> gcc/ada/
> 2012-03-14  Tristan Gingold  <gingold@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);
> 
> 
>
Tristan Gingold - March 14, 2012, 4:25 p.m.
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.

Will write a predicate in tree.[ch].

Thank you for your comments,
Tristan.
Richard Guenther - March 15, 2012, 9:37 a.m.
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.
Tristan Gingold - March 15, 2012, 12:51 p.m.
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.

Patch

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);