| Message ID | 875C6F56-7161-48DF-91FB-3A01891F8289@adacore.com |
|---|---|
| State | New |
| Headers | show |
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); > > >
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.
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.
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.
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);
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.