[fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

Dave Korn wrote:

>   This is now PR40309.  Looks almost trivially easy to fix for anyone familiar
> with the structure of the fortran frontend.

  Less so for someone like me who has no real clue about fortran internals,
though.  I see there are two places in fortran/parse.c that call
main_program_symbol(): either as a result of a PROGRAM statement

    case ST_PROGRAM:
      if (seen_program)
	goto duplicate_main;
      seen_program = 1;
      prog_locus = gfc_current_locus;

      push_state (&s, COMP_PROGRAM, gfc_new_block);
      main_program_symbol(gfc_current_ns, gfc_new_block->name);

... or if the code starts with a nameless block:

    /* Anything else starts a nameless main program block.  */
    default:
      if (seen_program)
	goto duplicate_main;
      seen_program = 1;
      prog_locus = gfc_current_locus;

      push_state (&s, COMP_PROGRAM, gfc_new_block);
      main_program_symbol (gfc_current_ns, "MAIN__");

  One thing I don't understand is why there is a function
gfc_sym_mangled_function_id() in trans-decl.c that has this code:

      /* Main program is mangled into MAIN__.  */
      if (sym->attr.is_main_program)
	return get_identifier ("MAIN__");

  Is this something to do with the relationship between fortran symbols and
their mangled assembler equivalents?  What happens if we have a named program
section that isn't called "MAIN__"?  Won't this return the wrong identifier?

  It appears that this or something else is causing both functions to have the
same DECL_NAME when they are passed to gimple_expand_cfg() in cfgexpand.c, so
they both get identified as the main function and potentially have static ctor
calls to __main() inserted.  This is because the testcase I'm looking at uses
a "program main" directive.  The two function decls have different underlying
sym_refs - "MAIN__" vs. "main" - but they both point to the same
IDENTIFIER_NODE, for "main" in their DECL_NAME fields.

  I think this is probably an invalid way for the front-end to drive the
mid-end - it's ok when the two functions are semantically the same, as when
C++ clones constructors, but these are actually two entirely different
functions, and in particular, only one of them should cause
expand_main_function to be called.  I'd like that to be the real "main"
function, which is where the fortran runtime init gets called, rather than
"MAIN__", which is the user-level main function, because the runtime init
itself might need to use st_printf and that won't work until __main() is
called, but I'm not sure how to disetangle the two now.

    cheers,
      DaveK

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Tobias Burnus]

Dave,

Dave Korn wrote:
> I see there are two places in fortran/parse.c
Side remark: You know that the actual middle-end TREE is only generated
in trans*.c? Thus, all things which happen before like parse.c can be
still remodeled in trans*c.

> that call
> main_program_symbol(): either as a result of a PROGRAM statement
> ... or if the code starts with a nameless block:
>   
The reason is one can write a Fortran main program either as:
---------------------------
print *, 'Hello World'
end
---------------------------
or as
---------------------------
program HelloWorld
   print *, 'Hello World'
end program HelloWorld
---------------------------

And both denotes a Fortran main program, which should end up under the
(assembler) name "MAIN__" in the .s file.

>   One thing I don't understand is why there is a function
> gfc_sym_mangled_function_id() in trans-decl.c that has this code:
>       /* Main program is mangled into MAIN__.  */
>       if (sym->attr.is_main_program)
> 	return get_identifier ("MAIN__");
>   
As for debugging messages etc. the name to the front end is, e.g.,
"HelloWorld", one needs to convert it into MAIN__ (which is for historic
reasons the assembler name of the Fortran main program in code generated
by several compilers, be it g77, g95, gfortran, ifort or sunf95).

> It appears that this or something else is causing both functions to have the
> same DECL_NAME when they are passed to gimple_expand_cfg() in cfgexpand.c, so
> they both get identified as the main function and potentially have static ctor
> calls to __main() inserted.  This is because the testcase I'm looking at uses
> a "program main" directive.  The two function decls have different underlying
> sym_refs - "MAIN__" vs. "main" - but they both point to the same
> IDENTIFIER_NODE, for "main" in their DECL_NAME fields.
>   
Hmm, that should not be the case that the middle end gets confused. I
think there is some merit in printing also the name, e.g., HelloWorld
rather than MAIN__ in middle end warnings ("unused variable foo in
HelloWorld"), but otherwise the name given to the program in gimple
shouldn't matter as long as the assembler name is MAIN__.

Seemingly there are some issues; however, they do not seem to be new.
MAIN__ was before generated and the main() was linked from the library.
The library's main (in fmain.c / libgfortranbegin.a) should already have
called __main(). Thus if gimple_expand_cfg() automatically adds __main()
calls for "main" then this must have happened before.

> I think this is probably an invalid way for the front-end to drive the
> mid-end - it's ok when the two functions are semantically the same, as when
> C++ clones constructors, but these are actually two entirely different
> functions, and in particular, only one of them should cause
> expand_main_function to be called.  I'd like that to be the real "main"
> function, which is where the fortran runtime init gets called, rather than
> "MAIN__", which is the user-level main function, because the runtime init
> itself might need to use st_printf and that won't work until __main() is
> called, but I'm not sure how to disetangle the two now.
>   

I agree that for "main" the call to "__main()" should happend and thus
expand_main_function should be called. I'm not sure in about the exact
assumptions of the middle end. In principle, it would be OK if the
MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
The only potential inconvenience I see, is the mentioned reference to
MAIN__ instead of <program name> in middle-end warnings, which can
confuse users.

Tobias

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

Tobias Burnus wrote:

  Hi Tobias,

> Hmm, that should not be the case that the middle end gets confused. I
> think there is some merit in printing also the name, e.g., HelloWorld
> rather than MAIN__ in middle end warnings ("unused variable foo in
> HelloWorld"), but otherwise the name given to the program in gimple
> shouldn't matter as long as the assembler name is MAIN__.
> 
> Seemingly there are some issues; however, they do not seem to be new.
> MAIN__ was before generated and the main() was linked from the library.
> The library's main (in fmain.c / libgfortranbegin.a) should already have
> called __main(). Thus if gimple_expand_cfg() automatically adds __main()
> calls for "main" then this must have happened before.
> 
>> I think this is probably an invalid way for the front-end to drive the
>> mid-end - it's ok when the two functions are semantically the same, as when
>> C++ clones constructors, but these are actually two entirely different
>> functions, and in particular, only one of them should cause
>> expand_main_function to be called.  I'd like that to be the real "main"
>> function, which is where the fortran runtime init gets called, rather than
>> "MAIN__", which is the user-level main function, because the runtime init
>> itself might need to use st_printf and that won't work until __main() is
>> called, but I'm not sure how to disetangle the two now.
>>   
> 
> I agree that for "main" the call to "__main()" should happend and thus
> expand_main_function should be called. I'm not sure in about the exact
> assumptions of the middle end. In principle, it would be OK if the
> MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
> The only potential inconvenience I see, is the mentioned reference to
> MAIN__ instead of <program name> in middle-end warnings, which can
> confuse users.

  Wouldn't the simplest thing be to rename the other main function - the
initialisation one that is automatically generated by create_main_function()?
 It could be called anything different we liked, and it's not user-visible, so
it ought to not be a problem to rename?

    cheers,
      DaveK

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

Dave Korn wrote:
> Tobias Burnus wrote:

>> I agree that for "main" the call to "__main()" should happend and thus
>> expand_main_function should be called. I'm not sure in about the exact
>> assumptions of the middle end. In principle, it would be OK if the
>> MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
>> The only potential inconvenience I see, is the mentioned reference to
>> MAIN__ instead of <program name> in middle-end warnings, which can
>> confuse users.
> 
>   Wouldn't the simplest thing be to rename the other main function - the
> initialisation one that is automatically generated by create_main_function()?
>  It could be called anything different we liked, and it's not user-visible, so
> it ought to not be a problem to rename?

  Argh, no.  Cygwin crt0 for one expects the entrypoint function to be called
_main in any language.  Hmmm.

    cheers,
      DaveK

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

Dave Korn wrote:
> Dave Korn wrote:
>> Tobias Burnus wrote:
> 
>>> I agree that for "main" the call to "__main()" should happend and thus
>>> expand_main_function should be called. I'm not sure in about the exact
>>> assumptions of the middle end. In principle, it would be OK if the
>>> MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
>>> The only potential inconvenience I see, is the mentioned reference to
>>> MAIN__ instead of <program name> in middle-end warnings, which can
>>> confuse users.

  Is it legitimate to have a space in an IDENTIFIER_NODE?  I have a cheeky idea:

$ svn diff
Index: trans-decl.c
===================================================================
--- trans-decl.c        (revision 147949)
+++ trans-decl.c        (working copy)
@@ -3859,7 +3859,8 @@
   tmp =  build_function_type_list (integer_type_node, integer_type_node,
                                   build_pointer_type (pchar_type_node),
                                   NULL_TREE);
-  ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
+  main_identifier_node = get_identifier ("main");
+  ftn_main = build_decl (FUNCTION_DECL, main_identifier_node, tmp);
   DECL_EXTERNAL (ftn_main) = 0;
   TREE_PUBLIC (ftn_main) = 1;
   TREE_STATIC (ftn_main) = 1;
Index: parse.c
===================================================================
--- parse.c     (revision 147949)
+++ parse.c     (working copy)
@@ -1424,8 +1424,10 @@
 {
   gfc_symbol *main_program;
   symbol_attribute attr;
+  const char *identifier;

-  gfc_get_symbol (name, ns, &main_program);
+  identifier = gfc_get_string ("PROGRAM %s", name);
+  gfc_get_symbol (identifier, ns, &main_program);
   gfc_clear_attr (&attr);
   attr.flavor = FL_PROGRAM;
   attr.proc = PROC_UNKNOWN;

DKAdmin@ubik /gnu/gcc/gcc-patched/gcc/fortran
$

  That should give reasonable warnings from the middle end, no?  I don't know
what it might do to debugging though.

    cheers,
      DaveK

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Tobias Burnus]

Dave Korn wrote:
> Dave Korn wrote:
>> Dave Korn wrote:
>>> Tobias Burnus wrote:
>>>> I agree that for "main" the call to "__main()" should happend and thus
>>>> expand_main_function should be called. I'm not sure in about the exact
>>>> assumptions of the middle end. In principle, it would be OK if the
>>>> MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
>>>> The only potential inconvenience I see, is the mentioned reference to
>>>> MAIN__ instead of <program name> in middle-end warnings, which can
>>>> confuse users.
> 
>   Is it legitimate to have a space in an IDENTIFIER_NODE?  I have a cheeky idea:

> -  gfc_get_symbol (name, ns, &main_program);
> +  identifier = gfc_get_string ("PROGRAM %s", name);

>   That should give reasonable warnings from the middle end, no?  I don't know
> what it might do to debugging though.

Currently one can use "b MAIN__" and "b helloworld" in the debugger:

(gdb) b helloworld
Breakpoint 1 at 0x400737: file test.f90, line 3.
(gdb) b MAIN__
Note: breakpoint 1 also set at pc 0x400737.
Breakpoint 2 at 0x400737: file test.f90, line 3.
(gdb) b main
Breakpoint 3 at 0x4007b9: file test.f90, line 9.

I have another idea: Just handle "PROGRAM main" specially by using the
name "MAIN__". It should be still quite readable in the middle-end
diagnostics. Furthermore, it matches the assembler name and using "b
main" one still get something useful - and it is less confusing for
everyone (middle end, users of -fdump-tree-original etc.) if there is
only a single "main".

Tobias


Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c    (Revision 148004)
+++ gcc/fortran/trans-decl.c    (Arbeitskopie)
@@ -289,7 +289,10 @@ gfc_get_label_decl (gfc_st_label * lp)
 static tree
 gfc_sym_identifier (gfc_symbol * sym)
 {
-  return (get_identifier (sym->name));
+  if (sym->attr.is_main_program && strcmp (sym->name, "main") == 0)
+    return (get_identifier ("MAIN__"));
+  else
+    return (get_identifier (sym->name));
 }


@@ -3874,6 +3877,8 @@ 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 (FUNCTION_DECL, main_identifier_node, tmp);
   ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
   DECL_EXTERNAL (ftn_main) = 0;
   TREE_PUBLIC (ftn_main) = 1;

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Jerry DeLisle]

Tobias Burnus wrote:
> Dave Korn wrote:
>> Dave Korn wrote:
>>> Dave Korn wrote:
>>>> Tobias Burnus wrote:
>>>>> I agree that for "main" the call to "__main()" should happend and thus
>>>>> expand_main_function should be called. I'm not sure in about the exact
>>>>> assumptions of the middle end. In principle, it would be OK if the
>>>>> MAIN__ function would show up as MAIN__ in gimple/-fdump-tree-original.
>>>>> The only potential inconvenience I see, is the mentioned reference to
>>>>> MAIN__ instead of <program name> in middle-end warnings, which can
>>>>> confuse users.
>>   Is it legitimate to have a space in an IDENTIFIER_NODE?  I have a cheeky idea:
> 
>> -  gfc_get_symbol (name, ns, &main_program);
>> +  identifier = gfc_get_string ("PROGRAM %s", name);
> 
>>   That should give reasonable warnings from the middle end, no?  I don't know
>> what it might do to debugging though.
> 
> Currently one can use "b MAIN__" and "b helloworld" in the debugger:
> 
> (gdb) b helloworld
> Breakpoint 1 at 0x400737: file test.f90, line 3.
> (gdb) b MAIN__
> Note: breakpoint 1 also set at pc 0x400737.
> Breakpoint 2 at 0x400737: file test.f90, line 3.
> (gdb) b main
> Breakpoint 3 at 0x4007b9: file test.f90, line 9.
> 
> I have another idea: Just handle "PROGRAM main" specially by using the
> name "MAIN__". It should be still quite readable in the middle-end
> diagnostics. Furthermore, it matches the assembler name and using "b
> main" one still get something useful - and it is less confusing for
> everyone (middle end, users of -fdump-tree-original etc.) if there is
> only a single "main".
> 
> Tobias
> 
> 
> Index: gcc/fortran/trans-decl.c
> ===================================================================
> --- gcc/fortran/trans-decl.c    (Revision 148004)
> +++ gcc/fortran/trans-decl.c    (Arbeitskopie)
> @@ -289,7 +289,10 @@ gfc_get_label_decl (gfc_st_label * lp)
>  static tree
>  gfc_sym_identifier (gfc_symbol * sym)o commit
>  {
> -  return (get_identifier (sym->name));
> +  if (sym->attr.is_main_program && strcmp (sym->name, "main") == 0)
> +    return (get_identifier ("MAIN__"));
> +  else
> +    return (get_identifier (sym->name));
>  }
> 
> 
> @@ -3874,6 +3877,8 @@ 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 (FUNCTION_DECL, main_identifier_node, tmp);
>    ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
>    DECL_EXTERNAL (ftn_main) = 0;
>    TREE_PUBLIC (ftn_main) = 1;
> 
> 
Tobias and Dave,

I tested the above on x86-64 Linux.  OK to commit.

Jerry

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Tobias Burnus]

Jerry DeLisle wrote:
> I tested the above on x86-64 Linux.  OK to commit.
Thanks for the review. Committed:

Sending        gcc/fortran/ChangeLog
Sending        gcc/fortran/trans-decl.c
Transmitting file data ..
Committed revision 148035.

Tobias

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

Tobias Burnus wrote:
> Jerry DeLisle wrote:
>> I tested the above on x86-64 Linux.  OK to commit.
> Thanks for the review. Committed:
> 
> Sending        gcc/fortran/ChangeLog
> Sending        gcc/fortran/trans-decl.c
> Transmitting file data ..
> Committed revision 148035.
> 
> Tobias

  Thank you both :)

    cheers,
      DaveK

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

Jerry DeLisle wrote:
> Tobias Burnus wrote:

>> @@ -3874,6 +3877,8 @@ 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 (FUNCTION_DECL, main_identifier_node, tmp);
>>    ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
>>    DECL_EXTERNAL (ftn_main) = 0;
>>    TREE_PUBLIC (ftn_main) = 1;
>>
>>
> Tobias and Dave,
> 
> I tested the above on x86-64 Linux.  OK to commit.

  I just took a second look at this.  We surely didn't mean to build two decls
and throw one away, did we?  I think the second assignment to ftn_main was
supposed to have been deleted when the middle argument was changed.  It looks
harmless but superfluous to me.  I'll just double-check that removing it
doesn't break anything.  Ok if so?

gcc/fortran/ChangeLog

	* trans-decl.c (create_main_function):  Don't build main decl twice.

    cheers,
      DaveK

[-- Attachment #2: fortran-double-decl-main.diff --]
[-- Type: text/x-c, Size: 520 bytes --]

Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 148276)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -3876,7 +3876,6 @@
 				   NULL_TREE);
   main_identifier_node = get_identifier ("main");
   ftn_main = build_decl (FUNCTION_DECL, main_identifier_node, tmp);
-  ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
   DECL_EXTERNAL (ftn_main) = 0;
   TREE_PUBLIC (ftn_main) = 1;
   TREE_STATIC (ftn_main) = 1;

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Tobias Burnus]

Dave Korn wrote:
>>> +  main_identifier_node = get_identifier ("main");
>>> +  ftn_main = build_decl (FUNCTION_DECL, main_identifier_node, tmp);
>>>    ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);
>>>       
> I just took a second look at this.  We surely didn't mean to build two decls
> and throw one away, did we?
Why have I always read

-   ftn_main = build_decl (FUNCTION_DECL, get_identifier ("main"), tmp);

although there was no "-"?


> I think the second assignment to ftn_main was supposed to have been
> deleted when the middle argument was changed. Ok if so?
Yes, of cause it should have been deleted. OK for the trunk and thanks
for spotting it!

Tobias

> gcc/fortran/ChangeLog
>
> * trans-decl.c (create_main_function): Don't build main decl twice.

Re: [fortran] Different FUNC_DECLS with the same DECL_NAME - MAIN__ and named PROGRAM main functions [was Re: gcc-4.5-20090528 is now available]

[Tobias Burnus]

Dave Korn wrote:
>>   Wouldn't the simplest thing be to rename the other main function - the
>> initialisation one that is automatically generated by create_main_function()?
>>  It could be called anything different we liked, and it's not user-visible, so
>> it ought to not be a problem to rename?
>>     
>
>   Argh, no.  Cygwin crt0 for one expects the entrypoint function to be called
> _main in any language.  Hmmm.
>   

In terms of -fdump-tree-original (all(?) other dumps place the assembler
name in parentheses) and for the special handling of "main" in the
middle end, having MAIN__ everywhere would be useful. Except for the
warnings; having

  test.f90: In function 'helloworld':
  test.f90:3: warning: 'i' is used uninitialized in this function

is better readable for the user than a

  test.f90: In function 'MAIN__':
  test.f90:2: warning: 'i' is used uninitialized in this function

Frankly, I don't have any idea how to solve the problem while retaining
the proper names in the middle-end diagnostics. But of cause on can
decide that the diagnostic output is less of a problem and live with
"MAIN__" in the middle-end diagnostics. (The front-end diagnostic
message are not effected.)

Tobias