Bug 40969 - [4.5 regression] Revision 150465 failed gfortran.dg/c_by_val_1.f
Summary: [4.5 regression] Revision 150465 failed gfortran.dg/c_by_val_1.f
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.5.0
: P3 normal
Target Milestone: ---
Assignee: Tobias Burnus
URL:
Keywords:
Depends on:
Blocks: 40949
  Show dependency treegraph
 
Reported: 2009-08-05 02:26 UTC by H.J. Lu
Modified: 2020-02-18 18:43 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2009-08-05 20:42:52


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2009-08-05 02:26:49 UTC
On Linux/ia64, revision 150465 gave:

FAIL: gfortran.dg/c_by_val_1.f  -O0  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O1  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O2  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O3 -fomit-frame-pointer  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O3 -fomit-frame-pointer -funroll-loops  execution test
FAIL: gfortran.dg/c_by_val_1.f  -O3 -g  execution test
FAIL: gfortran.dg/c_by_val_1.f  -Os  execution test

Revision 150406 is OK.
Comment 1 Tobias Burnus 2009-08-05 07:19:39 UTC
Works for me on x86-64-linux and seems also to work on several other systems according to the testresults mailing list.
(Only failure - your ia64 system: http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00487.html)

As I cannot reproduce it, can you provide more details?

 * * *

Side note: Can you change your tester to not attach the results as binary but as "text/...", cf. http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00484.html; that makes is quicker to reading the test results.
Comment 2 Paul Thomas 2009-08-05 08:41:09 UTC
(In reply to comment #1)
> Works for me on x86-64-linux and seems also to work on several other systems
> according to the testresults mailing list.

Works for me too at revision 150482 on x86_64/FC9

Cheers

Paul
Comment 3 H.J. Lu 2009-08-05 18:58:50 UTC
(In reply to comment #1)
> Works for me on x86-64-linux and seems also to work on several other systems
> according to the testresults mailing list.
> (Only failure - your ia64 system:
> http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00487.html)
> 
> As I cannot reproduce it, can you provide more details?

It is caused by revision 150465:

http://gcc.gnu.org/ml/gcc-cvs/2009-08/msg00145.html

It is run-time failure.

>  * * *
> 
> Side note: Can you change your tester to not attach the results as binary but
> as "text/...", cf. http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00484.html;
> that makes is quicker to reading the test results.

See PR 40704.

Comment 4 kargls 2009-08-05 19:06:27 UTC
(In reply to comment #3)
> (In reply to comment #1)
> > Works for me on x86-64-linux and seems also to work on several other systems
> > according to the testresults mailing list.
> > (Only failure - your ia64 system:
> > http://gcc.gnu.org/ml/gcc-testresults/2009-08/msg00487.html)
> > 
> > As I cannot reproduce it, can you provide more details?
> 
> It is caused by revision 150465:
> 
> http://gcc.gnu.org/ml/gcc-cvs/2009-08/msg00145.html
> 
> It is run-time failure.

You have got to be kidding!  This is not the details that
was requested.  Your subject line already pointed to the
revision.  Where the heck is the backtrace?  What's the
difference in generated assembly before and after the
revision?
Comment 5 H.J. Lu 2009-08-05 19:07:52 UTC
(gdb) bt
#0  0xa000000000010621 in __kernel_syscall_via_break ()
#1  0x20000000003e7630 in raise () from /lib/tls/libc.so.6.1
#2  0x20000000003ea010 in abort () from /lib/tls/libc.so.6.1
#3  0x40000000000014b0 in f_to_f__ (retval=0x60000fffffffb420, a1=0, 
    a2=0x60000fffffffb430, a3=0x60000fffffffb418)
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val.c:21
#4  0x4000000000000ad0 in c_by_val_1 ()
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val_1.f:18
#5  0x40000000000013e0 in main (argc=1, 
    argv=0x60000fffffffb99c "/export/build/gnu/gcc/build-ia64-linux/gcc/testsuite/gfortran/c_by_val_1.exe")
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val_1.f:52
#6  0x20000000003bd430 in __libc_start_main () from /lib/tls/libc.so.6.1
#7  0x40000000000008a0 in _start ()
(gdb) f 3
#3  0x40000000000014b0 in f_to_f__ (retval=0x60000fffffffb420, a1=0, 
    a2=0x60000fffffffb430, a3=0x60000fffffffb418)
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val.c:21
21	  if ( a1 != *a2 ) abort();
(gdb) 
Comment 6 H.J. Lu 2009-08-05 19:09:02 UTC
21	  if ( a1 != *a2 ) abort();
(gdb) p a1
$1 = 0
(gdb) p *a2
$2 = 42
(gdb) 
Comment 7 H.J. Lu 2009-08-05 19:22:52 UTC
When we see

      external   f_to_f, i_to_i, c_to_c
      external   f_to_f8, i_to_i8, c_to_c8

and do

typelist = gfc_chainon_list (typelist, void_type_node);

Later, we see

call  f_to_f (b, %VAL (a), %REF (c), %LOC (c))

We build typelist again with an extra void_type_node at the beginning.
Comment 8 H.J. Lu 2009-08-05 19:28:37 UTC
(In reply to comment #7)
> When we see
> 
>       external   f_to_f, i_to_i, c_to_c
>       external   f_to_f8, i_to_i8, c_to_c8
> 
> and do
> 
> typelist = gfc_chainon_list (typelist, void_type_node);
> 
> Later, we see
> 
> call  f_to_f (b, %VAL (a), %REF (c), %LOC (c))
> 
> We build typelist again with an extra void_type_node at the beginning.
> 

That is incorrect. For some reason, typelist is always NULL before the change.
Comment 9 Tobias Burnus 2009-08-05 19:33:47 UTC
(In reply to comment #6)
> 21        if ( a1 != *a2 ) abort();
> (gdb) p a1
> $1 = 0
> (gdb) p *a2
> $2 = 42

Thanks! I think the following patch should cure this. I think we will still have issues with LTO but those are inevitable and need to be solved on the middle end.

Index: trans-types.c
===================================================================
--- trans-types.c       (Revision 150497)
+++ trans-types.c       (Arbeitskopie)
@@ -2324,7 +2324,10 @@
   while (nstr--)
     typelist = gfc_chainon_list (typelist, gfc_charlen_type_node);

-  typelist = gfc_chainon_list (typelist, void_type_node);
+  /* If the explicit interface is known, we tell the middle end
+     that no more additional arguments will follow in calls.  */
+  if (sym->attr.if_source != IFSRC_UNKNOWN)
+    typelist = gfc_chainon_list (typelist, void_type_node);

   if (alternate_return)
     type = integer_type_node;
Comment 10 H.J. Lu 2009-08-05 19:49:01 UTC
(In reply to comment #9)

> Thanks! I think the following patch should cure this. I think we will still
> have issues with LTO but those are inevitable and need to be solved on the
> middle end.
> 
> Index: trans-types.c
> ===================================================================
> --- trans-types.c       (Revision 150497)
> +++ trans-types.c       (Arbeitskopie)
> @@ -2324,7 +2324,10 @@
>    while (nstr--)
>      typelist = gfc_chainon_list (typelist, gfc_charlen_type_node);
> 
> -  typelist = gfc_chainon_list (typelist, void_type_node);
> +  /* If the explicit interface is known, we tell the middle end
> +     that no more additional arguments will follow in calls.  */
> +  if (sym->attr.if_source != IFSRC_UNKNOWN)
> +    typelist = gfc_chainon_list (typelist, void_type_node);
> 
>    if (alternate_return)
>      type = integer_type_node;
> 

It failed at a different place:

(gdb) bt
#0  0xa000000000010621 in __kernel_syscall_via_break ()
#1  0x20000000003e7630 in raise () from /lib/tls/libc.so.6.1
#2  0x20000000003ea010 in abort () from /lib/tls/libc.so.6.1
#3  0x4000000000001ac0 in c_to_c__ (retval=0x60000fffffffb498, c1=43 + 0 * I, 
    c2=0x60000fffffffb480, c3=0x60000fffffffb3e8)
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val.c:61
#4  0x4000000000000f50 in c_by_val_1 ()
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val_1.f:42
#5  0x4000000000001430 in main (argc=1, 
    argv=0x60000fffffffb983 "/export/build/gnu/gcc-work/build-ia64-linux/gcc/testsuite/gfortran/c_by_val_1.exe")
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val_1.f:52
#6  0x20000000003bd430 in __libc_start_main () from /lib/tls/libc.so.6.1
#7  0x40000000000008a0 in _start ()
(gdb) f 3
#3  0x4000000000001ac0 in c_to_c__ (retval=0x60000fffffffb498, c1=43 + 0 * I, 
    c2=0x60000fffffffb480, c3=0x60000fffffffb3e8)
    at /net/gnu-13/export/gnu/src/gcc/gcc/gcc/testsuite/gfortran.dg/c_by_val.c:61
61	  if ( c1 != *c2    ) abort();
(gdb) p c1
$1 = 43 + 0 * I
(gdb) p *c2
$2 = -1 + 2 * I
(gdb) 
Comment 11 H.J. Lu 2009-08-05 19:52:19 UTC
This patch works:

--- ./trans-types.c.foo	2009-08-05 07:26:53.000000000 -0700
+++ ./trans-types.c	2009-08-05 12:51:00.000000000 -0700
@@ -2324,7 +2324,10 @@ gfc_get_function_type (gfc_symbol * sym)
   while (nstr--)
     typelist = gfc_chainon_list (typelist, gfc_charlen_type_node);
 
-  typelist = gfc_chainon_list (typelist, void_type_node);
+  /* If the explicit interface is known, we tell the middle end
+     that no more additional arguments will follow in calls.  */
+  if (typelist || sym->attr.if_source != IFSRC_UNKNOWN)
+    typelist = gfc_chainon_list (typelist, void_type_node);
 
   if (alternate_return)
     type = integer_type_node;
Comment 12 Tobias Burnus 2009-08-05 20:03:05 UTC
(In reply to comment #10)
> It failed at a different place:
> 61        if ( c1 != *c2    ) abort();
> (gdb) p c1
> $1 = 43 + 0 * I
> (gdb) p *c2
> $2 = -1 + 2 * I

Hmm, I honestly have no idea why this fails. In my understanding, it should have failed before and should now be working. The reason is that for

      call  f_to_f (b, %VAL (a), %REF (c), %LOC (c))

using "external f_to_f" generates a prototype

      void f_to_f(...)

which my previous patch in PR 40949 wrongly modified to "void f_to_f(void)". However, for

      v = c_to_c (%VAL (u), %REF (w), %LOC (w))
with
      external c_to_c
      complex  c_to_c
and call by reference (-ff2c), the prototype should be

      void c_to_c(complex*, ...)

In my opinion that's what my patch in comment 9 does, while  pre-PR40949 gfortran appended a void node, leading to

      void c_to_c(complex *) // misses ", ..."

I think I miss something "obvious" here.

 * * *

At the end, gfortran should produce a proper prototype, which is useful to have to provide better diagnostics and needed to fix some LTO issues. I filled PR 40976 to track this.
Comment 13 Tobias Burnus 2009-08-05 20:36:27 UTC
(In reply to comment #11)
> This patch works:
> +  if (typelist || sym->attr.if_source != IFSRC_UNKNOWN)
> +    typelist = gfc_chainon_list (typelist, void_type_node);

That patch essentially undoes the patch of PR 40949 as this is essentially identically to

   if (typelist)
     typelist = gfc_chainon_list (typelist, void_type_node);

I think it is best to go back to the old version (i.e. revert the patch PR 40949) and fix it properly as proposed in PR 40976 (and in PR 40978)
Comment 14 Tobias Burnus 2009-08-05 20:47:34 UTC
Subject: Bug 40969

Author: burnus
Date: Wed Aug  5 20:47:19 2009
New Revision: 150500

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=150500
Log:
2009-08-05  Tobias Burnus  <burnus@net-b.de>

        PR fortran/40969
        Revert:
        2009-08-04  Tobias Burnus  <burnus@net-b.de>

        PR fortran/40949
        * trans-types.c (gfc_get_function_type): Fix typelist of
        functions without argument.


Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-types.c

Comment 15 Tobias Burnus 2009-08-05 20:52:01 UTC
FIXED by reverting the bug.