User account creation filtered due to spam.

Bug 53718 - [4.7 regression] [OOP] gfortran generates asm label twice in the same output file
Summary: [4.7 regression] [OOP] gfortran generates asm label twice in the same output ...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: fortran (show other bugs)
Version: 4.7.1
: P4 normal
Target Milestone: 4.8.0
Assignee: janus
URL:
Keywords: rejects-valid, wrong-code
Depends on:
Blocks:
 
Reported: 2012-06-18 17:42 UTC by Adrian Prantl
Modified: 2014-06-12 13:33 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 4.8.3
Known to fail: 4.7.4
Last reconfirmed: 2012-06-18 00:00:00


Attachments
Delta-reduced input code (383 bytes, application/octet-stream)
2012-06-18 17:42 UTC, Adrian Prantl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Prantl 2012-06-18 17:42:51 UTC
Created attachment 27653 [details]
Delta-reduced input code

gfortran --version
GNU Fortran (GCC) 4.7.1
Copyright (C) 2012 Free Software Foundation, Inc.

uname -a
Linux [...] 2.6.32-41-generic #89-Ubuntu SMP Fri Apr 27 22:18:56 UTC 2012 x86_64 GNU/Linux

gfortran  -c exceptionstest.f03
/tmp/cc95FYwH.s: Assembler messages:
/tmp/cc95FYwH.s:148: Error: symbol `__copy_idl_int_array_f03_Idl_int_2d' is already defined
/tmp/cc95FYwH.s:199: Error: symbol `__def_init_idl_int_array_f03_Idl_int_2d' is already defined
/tmp/cc95FYwH.s:203: Error: symbol `__vtab_idl_int_array_f03_Idl_int_2d' is already defined

Delta-reduced input code is attached.
Comment 1 janus 2012-06-18 18:46:10 UTC
Confirmed with 4.7 and trunk. This is actually a regression, since it works with 4.6.3.
Comment 2 Adrian Prantl 2012-06-18 18:55:11 UTC
just as a side note: most of my bugreports come from compiling babel's regression test suite. Babel is a language interoperability tool that (also) generates bindings for Fortran 2003. It's free (LGPL) and it might be a very good testcase for gfortran in the future.

https://computation.llnl.gov/casc/components/
Comment 3 janus 2012-06-18 20:04:09 UTC
(In reply to comment #2)
> just as a side note: most of my bugreports come from compiling babel's
> regression test suite. Babel is a language interoperability tool that (also)
> generates bindings for Fortran 2003. It's free (LGPL) and it might be a very
> good testcase for gfortran in the future.
> 
> https://computation.llnl.gov/casc/components/

Thanks for the pointer. Sounds interesting.


Btw, here is a slightly different (but much simpler) test case, which exhibits the same problem:


module m
  type t
  end type
end module

subroutine sub1
  use m
  class(t), pointer :: a1
end subroutine

subroutine sub2
  use m
  class(t), pointer :: a2
end subroutine


This structure is rather unusual for F03 object-oriented code, since it involves 'naked' subroutines (F77 style), which are not part of any MODULE or PROGRAM. Putting both routines into a module or program makes the error go away.
Comment 4 Dominique d'Humieres 2012-06-18 20:35:51 UTC
Revision 181425 (2011-11-16) is OK,
revision 181881 (2011-12-01) is not.
Comment 5 Dominique d'Humieres 2012-06-18 20:45:18 UTC
Could it be revision 181505?
Comment 6 janus 2012-06-19 10:35:34 UTC
(In reply to comment #5)
> Could it be revision 181505?

Very likely. If it is, I'm betting on the PR50640 part of that commit.
Comment 7 janus 2012-06-19 13:13:03 UTC
(In reply to comment #6)
> > Could it be revision 181505?
> 
> Very likely. If it is, I'm betting on the PR50640 part of that commit.

Indeed the following patch, which is practically a revert of the trans-decl.c part of the above commit, makes the errors go away:


Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 188334)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -1483,7 +1483,6 @@ gfc_get_symbol_decl (gfc_symbol * sym)
       || (sym->name[0] == '_' && strncmp ("__def_init", sym->name, 10) == 0))
     {
       TREE_READONLY (decl) = 1;
-      GFC_DECL_PUSH_TOPLEVEL (decl) = 1;
     }
 
   return decl;
@@ -1913,8 +1912,7 @@ build_function_decl (gfc_symbol * sym, bool global
   /* Layout the function declaration and put it in the binding level
      of the current function.  */
 
-  if (global
-      || (sym->name[0] == '_' && strncmp ("__copy", sym->name, 6) == 0))
+  if (global)
     pushdecl_top_level (fndecl);
   else
     pushdecl (fndecl);
Comment 8 Dominique d'Humieres 2012-06-19 17:36:26 UTC
(In reply to comment #7)
> Indeed the following patch, which is practically a revert of the trans-decl.c
> part of the above commit, makes the errors go away: ...

Confirmed for the tests in this PR (as well as the test in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46313#c25 ) without regression.

Note that without the patch the tests fail only with -O0, but not with the optimization I have tried (-O1 or -O3).

It would be nice if someone could compile gfortran.dg/select_type_12.f03 (and the tests) through valgrind (I cannot do it under darwin).
Comment 9 janus 2012-06-20 09:19:28 UTC
(In reply to comment #8)
> It would be nice if someone could compile gfortran.dg/select_type_12.f03 (and
> the tests) through valgrind (I cannot do it under darwin).

While valgrind 3.6.1 gives me some kind of internal error (cf. https://bugs.kde.org/show_bug.cgi?id=277045), valgrind 3.7.0 seems to work well and produces the following output:


> valgrind `gfortran-4.8 -v select_type_12.f03 2>&1 | grep f951`
==32473== Memcheck, a memory error detector
==32473== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==32473== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==32473== Command: /usr/lib/gcc/x86_64-unknown-linux-gnu/4.8.0/f951 select_type_12.f03 -quiet -dumpbase select_type_12.f03 -mtune=generic -march=x86-64 -auxbase select_type_12 -version -fintrinsic-modules-path /usr/lib64/gcc/x86_64-unknown-linux-gnu/4.8.0/finclude -o /tmp/ccOu2tOb.s
==32473== 
GNU Fortran (GCC) version 4.8.0 20120620 (experimental) [trunk revision 188819] (x86_64-unknown-linux-gnu)
        compiled by GNU C version 4.6.3 20120531 [gcc-4_6-branch revision 188067], GMP version 5.0.5, MPFR version 3.1.0-p1, MPC version 0.8.2
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU Fortran (GCC) version 4.8.0 20120620 (experimental) [trunk revision 188819] (x86_64-unknown-linux-gnu)
        compiled by GNU C version 4.6.3 20120531 [gcc-4_6-branch revision 188067], GMP version 5.0.5, MPFR version 3.1.0-p1, MPC version 0.8.2
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
==32473== 
==32473== HEAP SUMMARY:
==32473==     in use at exit: 425,388 bytes in 1,978 blocks
==32473==   total heap usage: 9,348 allocs, 7,370 frees, 4,853,954 bytes allocated
==32473== 
==32473== LEAK SUMMARY:
==32473==    definitely lost: 15,827 bytes in 90 blocks
==32473==    indirectly lost: 18,024 bytes in 62 blocks
==32473==      possibly lost: 192 bytes in 1 blocks
==32473==    still reachable: 391,345 bytes in 1,825 blocks
==32473==         suppressed: 0 bytes in 0 blocks


So, apparently there are no errors.

Moreover, the patch seems to survive a regtest on x86_64-unknown-linux-gnu without any failures in the gfortran testsuite.
Comment 10 Tobias Burnus 2012-09-11 13:47:59 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > > Could it be revision 181505?
> > Very likely. If it is, I'm betting on the PR50640 part of that commit.
> 
> Indeed the following patch, which is practically a revert of the trans-decl.c
> part of the above commit, makes the errors go away:

We should backout everything - except for the class.c part of the commit in bug 50640 comment 24.

My suspicious is that one of Richard's commits in May fixed the issue. In turn that probably means that backing out the patch for PR50640 only works for 4.7 and not for 4.8

See also some comments/analysis/RFC at bug 50640 comment 26.
Comment 11 janus 2012-09-11 15:57:35 UTC
(In reply to comment #10)
> My suspicious is that one of Richard's commits in May fixed the issue. In turn
> that probably means that backing out the patch for PR50640 only works for 4.7
> and not for 4.8

I assume you mean the other way around, right? The patch of comment 7 *does* work on trunk. I just checked that applying it on the 4.7 branch revives the old ICE on select_type_12.

So, should we apply the patch only on trunk for now?
Comment 12 Jakub Jelinek 2012-09-20 10:18:09 UTC
GCC 4.7.2 has been released.
Comment 13 janus 2012-10-31 21:56:00 UTC
Author: janus
Date: Wed Oct 31 21:55:50 2012
New Revision: 193048

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193048
Log:
2012-10-31  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/53718
	* trans.h (GFC_DECL_PUSH_TOPLEVEL): Removed.
	* trans-decl.c (gfc_get_symbol_decl,gfc_generate_function_code): Remove
	GFC_DECL_PUSH_TOPLEVEL.
	(build_function_decl): Do not push __copy procedure to toplevel.

2012-10-31  Janus Weil  <janus@gcc.gnu.org>

	PR fortran/53718
	* gfortran.dg/class_54.f90: New.

Added:
    trunk/gcc/testsuite/gfortran.dg/class_54.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans.h
    trunk/gcc/testsuite/ChangeLog
Comment 14 janus 2012-10-31 22:03:39 UTC
r193048 fixes the regression on trunk. Unfortunately it cannot be backported to 4.7.
Comment 15 janus 2013-01-27 21:31:48 UTC
The only way I can see to fix this on the 4.7 branch would be to identify the commit which fixed things on the 4.8 trunk (cf. comment 10), and backport it to 4.7 (if possible).

OTOH, I don't think this bug is extremely critical: It has a simple workaround, which is to put stuff into modules (this is 'good style' anyway, even more so for object-oriented code). So, maybe WONTFIX for 4.7?
Comment 16 Richard Biener 2013-04-11 07:59:18 UTC
GCC 4.7.3 is being released, adjusting target milestone.
Comment 17 Richard Biener 2014-06-12 13:12:42 UTC
Fixed for 4.8.0?
Comment 18 Dominique d'Humieres 2014-06-12 13:33:51 UTC
> Fixed for 4.8.0?

I cannot say for 4.8.0, but it is fixed with 4.8.3.