[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #17 from burnus at gcc dot gnu dot org 2010-04-06 12:47 --- Subject: Bug 43178 Author: burnus Date: Tue Apr 6 12:46:19 2010 New Revision: 157993 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=157993 Log: 2010-04-06 Tobias Burnus bur...@net-b.de PR fortran/43178 * trans-array.c (gfc_conv_expr_descriptor): Update gfc_trans_scalar_assign call. (has_default_initializer): New function. (gfc_trans_deferred_array): Nullify less often. * trans-expr.c (gfc_conv_subref_array_arg, gfc_trans_subcomponent_assign): Update call to gfc_trans_scalar_assign. (gfc_trans_scalar_assign): Add parameter and pass it on. (gfc_trans_assignment_1): Optionally, do not dealloc before assignment. * trans-openmp.c (gfc_trans_omp_array_reduction): Update call to gfc_trans_scalar_assign. * trans-decl.c (gfc_get_symbol_decl): Do not always apply initializer to static variables. (gfc_init_default_dt): Add dealloc parameter and pass it on. * trans-stmt.c (forall_make_variable_temp, generate_loop_for_temp_to_lhs, generate_loop_for_rhs_to_temp, gfc_trans_forall_1, gfc_trans_where_assign, gfc_trans_where_3 gfc_trans_allocate): Update gfc_trans_assignment call. * trans.h (gfc_trans_scalar_assign, gfc_init_default_dt, gfc_init_default_dt, gfc_trans_assignment): Add bool dealloc parameter to prototype. 2010-04-06 Tobias Burnus bur...@net-b.de PR fortran/43178 * gfortran.dg/alloc_comp_basics_1.f90: Update * scan-tree-dump-times. * gfortran.dg/alloc_comp_constructor_1.f90: Ditto. * gfortran.dg/auto_dealloc_1.f90: Ditto. Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/trans-array.c trunk/gcc/fortran/trans-decl.c trunk/gcc/fortran/trans-expr.c trunk/gcc/fortran/trans-openmp.c trunk/gcc/fortran/trans-stmt.c trunk/gcc/fortran/trans.h trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gfortran.dg/alloc_comp_basics_1.f90 trunk/gcc/testsuite/gfortran.dg/alloc_comp_constructor_1.f90 trunk/gcc/testsuite/gfortran.dg/auto_dealloc_1.f90 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #18 from burnus at gcc dot gnu dot org 2010-04-06 12:48 --- FIXED on the trunk (GCC 4.6). -- burnus at gcc dot gnu dot org changed: What|Removed |Added Status|NEW |RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #16 from pault at gcc dot gnu dot org 2010-03-27 18:53 --- What is the status of this one Tobias? I'll confirm it whilst I'm here :-) Paul -- pault at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2010-03-27 18:53:09 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #13 from dominiq at lps dot ens dot fr 2010-02-28 11:20 --- Thanks for testing! In trans-array.c's gfc_trans_deferred_array, my current version has - if (sym-value) + if (sym-value == NULL || !has_default_initializer (sym-ts.u.derived)) ^^ This part is new Hopefully, that is all what is needed. Apparently, yes (all my tests pass now). Following pr43205, I have found an other issue. I don't see this pr with patched trunk, but with all my other builds. So I have slightly changed the test to subroutine testproblem integer nsub_sta(1146, 2000), nsub_sto(1146, 2000) write (*,*) nsub_sta(1000,1000) write (*,*) nsub_sto(1000,1000) return end call testproblem end With revision 156618 I get [macbook] f90/bug% time gfc_c -fno-automatic -finit-integer=-100 pr43205_db.f90 4.575u 0.463s 0:06.59 76.3% 0+0k 1+25io 0pf+0w [macbook] f90/bug% a.out -100 -100 but with my patched trunk I get [macbook] f90/bug% time gfc -fno-automatic -finit-integer=-100 pr43205_db.f90 0.019u 0.022s 0:00.04 75.0% 0+0k 0+0io 0pf+0w [macbook] f90/bug% a.out 0 0 [macbook] f90/bug% time gfc -finit-integer=-100 pr43205_db.f90 0.022u 0.022s 0:00.05 80.0% 0+0k 0+8io 0pf+0w [macbook] f90/bug% a.out -100 -100 So the patch breaks -finit-integer when used with -fno-automatic. Still, the patch needs to be audited - I wouldn't be surprised if some free were missing, leading to memory leakage in the generated code. /* Subroutine of gfc_trans_assignment that actually scalarizes the - assignment. EXPR1 is the destination/LHS and EXPR2 is the source/RHS. */ + assignment. EXPR1 is the destination/LHS and EXPR2 is the source/RHS. + init_flag indicates initialization expressions, no_free that no ^^^ + deallocate prior assignment is needed (if in doubt, set true). */ static tree -gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag) +gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr * expr2, bool init_flag, + bool dealloc) ^^^ It seems that no_free has been changed to dealloc. My knowledge of gfortran is too bad to allow me to do any audit beyond this point!-( -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #14 from burnus at gcc dot gnu dot org 2010-02-28 17:30 --- Created an attachment (id=19988) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19988action=view) Another update - handle -fno-automatic fixes ICE in comment #11 Fixes ICE in comment #11, handles STATIC implied by -fno-automatic. Regarding no_free - dealloc: The change was done because !no_free (read: no(t) no_free sounds awkward. no-dealloc(ate) sounds better, albeit not much. And unfortunately, the compiler cannot compile-time diagnose such inconsistencies in comments ;-) (Now fixed.) Regarding the failure in comment #13: I missed a check for -fno-automatic, which also implies SAVE; this is now fixed. Regarding the (de|in)creased compile time: This is unavoidable in the current implementation, see PR 43210 for a possible fix. -- burnus at gcc dot gnu dot org changed: What|Removed |Added Attachment #19972|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #15 from burnus at gcc dot gnu dot org 2010-02-28 19:48 --- Created an attachment (id=19989) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19989action=view) Really fix -fno-automatic -- attached the wrong patch which missed a == 0 in trans-decl. As pointed out by Dominique. Next step is to proofread the patch, check the dealloc argument and check for run-time leakage of the testsuite's dg-do run tests. Re-checking some dumps cannot harm either. -- burnus at gcc dot gnu dot org changed: What|Removed |Added Attachment #19988|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #11 from dominiq at lps dot ens dot fr 2010-02-27 10:35 --- With the patch in comment #10, the tests in pr40440 (the original one and the one in comment #2 give an ICE: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0x gfc_trans_assignment (expr1=0x142a32110, expr2=0x0, init_flag=0 '\0', dealloc=0 '\0') at ../../work/gcc/fortran/trans-expr.c:5380 5380 if (expr2-expr_type == EXPR_FUNCTION expr2-rank 0) (gdb) bt #0 gfc_trans_assignment (expr1=0x142a32110, expr2=0x0, init_flag=0 '\0', dealloc=0 '\0') at ../../work/gcc/fortran/trans-expr.c:5380 #1 0x0001000c2304 in gfc_init_default_dt (sym=0x142a0a170, body=0x0, dealloc=0 '\0') at ../../work/gcc/fortran/trans-decl.c:3011 #2 0x0001000b1810 in gfc_trans_deferred_array (sym=0x142a0a170, body=0x1429dc820) at ../../work/gcc/fortran/trans-array.c:6258 #3 0x0001000c2aa1 in gfc_trans_deferred_vars (proc_sym=0x1428fbb60, fnbody=value temporarily unavailable, due to optimizations) at ../../work/gcc/fortran/trans-decl.c:3159 #4 0x0001000c3be4 in gfc_generate_function_code (ns=value temporarily unavailable, due to optimizations) at ../../work/gcc/fortran/trans-decl.c:4400 #5 0x0001000a79eb in gfc_generate_module_code (ns=value temporarily unavailable, due to optimizations) at ../../work/gcc/fortran/trans.c:1382 #6 0x00010006955f in gfc_parse_file () at ../../work/gcc/fortran/parse.c:4228 #7 0x0001000a27fc in gfc_be_parse_file (set_yydebug=value temporarily unavailable, due to optimizations) at ../../work/gcc/fortran/f95-lang.c:239 #8 0x0001006d5c9a in toplev_main (argc=2, argv=0x7fff5fbfda18) at ../../work/gcc/toplev.c:1053 #9 0x000110a4 in start () Otherwise all my other tests passed and regression testing went fine. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #12 from burnus at gcc dot gnu dot org 2010-02-27 12:24 --- (In reply to comment #11) With the patch in comment #10, the tests in pr40440 (the original one and the one in comment #2 give an ICE: Thanks for testing! In trans-array.c's gfc_trans_deferred_array, my current version has - if (sym-value) + if (sym-value == NULL || !has_default_initializer (sym-ts.u.derived)) ^^ This part is new Hopefully, that is all what is needed. Still, the patch needs to be audited - I wouldn't be surprised if some free were missing, leading to memory leakage in the generated code. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #3 from dominiq at lps dot ens dot fr 2010-02-26 14:30 --- With the patch in comment #2, I see a dozen runtime failure on my tests. I'll need some time to analyse them (I have to separate invalid codes that pass by chance from valid code that are miscompiled). So keep tuned!-) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #4 from dominiq at lps dot ens dot fr 2010-02-26 14:38 ---
The first obvious wrong code is for gcc/testsuite/gfortran.dg/streamio_6.f90:
The original dump without the patch shows
static integer(kind=4) a[100] = {13, 9, 34, 41, 25, 98, 6, 12, 11, 44, 79,
3, 64, 61, 77, 57, 59, 2, 92, 38, 71, 64, 31, 60, 28, 90, 26, 97, 47, 26, 48,
96, 95, 82, 100, 90, 45, 71, 71, 67, 72, 76, 94, 49, 85, 45, 100, 22, 96, 48,
13, 23, 40, 14, 76, 99, 96, 90, 65, 2, 8, 60, 96, 19, 45, 1, 100, 48, 91, 20,
92, 72, 81, 59, 24, 37, 43, 21, 54, 68, 31, 19, 79, 63, 41, 42, 12, 10, 62, 43,
9, 30, 9, 54, 35, 4, 5, 55, 3, 94};
but without the patch a[100] is not intialized
static integer(kind=4) a[100];
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #5 from dominiq at lps dot ens dot fr 2010-02-26 16:29 --- Another failing test is gcc/testsuite/gfortran.dg/data_char_2.f90: without patch static character(kind=1) intstr[1:10] = 0123456789; with patch static character(kind=1) intstr[1:10]; Note that in order to avoid any side effect with other patches, I have applied the patch on a clean trunk at revision 157090. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #6 from burnus at gcc dot gnu dot org 2010-02-26 16:37 ---
(In reply to comment #4)
The first obvious wrong code is for gcc/testsuite/gfortran.dg/streamio_6.f90:
but without the patch a[100] is not intialized
static integer(kind=4) a[100];
In trans-decl.c:
if (TREE_STATIC (decl) !sym-attr.use_assoc
(sym-attr.save || sym-attr.is_main_program))
{
Change attr.is_main_program to sym-ns-proc_name-attr.is_main_program
That happens with late morning changes, when one wants to urgently leave for
work ...
Without the change you will get tons of errors for variables initialized in
PROGRAM - something particularly common in the test suite.
As I wrote this morning: I have not regtested it, but I think it is very
unlikely that it fails :-)
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #7 from dominiq at lps dot ens dot fr 2010-02-26 19:27 --- Change attr.is_main_program to sym-ns-proc_name-attr.is_main_program This change fixes most of the failures I have seen. Is if (TREE_STATIC (decl) !sym-attr.use_assoc (sym-attr.save || sym-attr.is_main_program)) the only place where this change should be done? The following test still fails: implicit none CALL data_init_scalar() CONTAINS SUBROUTINE data_init_scalar() integer :: a data a / 1 / IF (a /= 1) CALL abort() END SUBROUTINE END and some ICEs related to overlapping intializations (with DATA statements?) are gone. also the test in comment#2 of pr40440 (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17996action=view ) abort at run time with Starting to load ifile a.out(29469) malloc: *** error for object 0xff00: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug Abort I am now starting a gfortran regtest. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #8 from dominiq at lps dot ens dot fr 2010-02-26 19:37 --- First failures FAIL: gfortran.dg/auto_dealloc_1.f90 -O scan-tree-dump-times original __builtin_free 5 - should be 4 FAIL: gfortran.dg/common_resize_1.f -O0 execution test FAIL: gfortran.dg/common_resize_1.f -O1 execution test FAIL: gfortran.dg/common_resize_1.f -O2 execution test FAIL: gfortran.dg/common_resize_1.f -O3 -fomit-frame-pointer execution test FAIL: gfortran.dg/common_resize_1.f -O3 -fomit-frame-pointer -funroll-loops execution test FAIL: gfortran.dg/common_resize_1.f -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test FAIL: gfortran.dg/common_resize_1.f -O3 -g execution test FAIL: gfortran.dg/common_resize_1.f -Os execution test -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #9 from dominiq at lps dot ens dot fr 2010-02-26 21:07 --- Additional failures (both -m32 and -m64) FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O0 FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O1 FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O2 FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O2 -fomit-frame-pointer -finline-functions FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O2 -fomit-frame-pointer -finline-functions -funroll-loops FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O2 -fbounds-check FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O3 -g FAIL: gfortran.fortran-torture/execute/data.f90 execution, -Os FAIL: gfortran.fortran-torture/execute/data.f90 execution, -O2 -ftree-vectorize -msse2 FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O0 FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O1 FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O2 FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O2 -fomit-frame-pointer -finline-functions FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O2 -fomit-frame-pointer -finline-functions -funroll-loops FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O2 -fbounds-check FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O3 -g FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -Os FAIL: gfortran.fortran-torture/execute/module_init_1.f90 execution, -O2 -ftree-vectorize -msse2 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #10 from burnus at gcc dot gnu dot org 2010-02-26 23:19 --- Created an attachment (id=19972) -- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19972action=view) Next try - regtested, but not poofread -- burnus at gcc dot gnu dot org changed: What|Removed |Added Attachment #19966|0 |1 is obsolete|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #1 from burnus at gcc dot gnu dot org 2010-02-25 23:51 ---
Created an attachment (id=19962)
-- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19962action=view)
Draft patch - regtests, but needs some audit
The attached patch drastically reduces the generated code
(-fdump-tree-original) for initializations.
a) For variables in PROGRAM which are static (note: variables in PROGRAM are
implicitly SAVE).
b) For all kind of derived types with default initializer, especially those
which have allocatable components
I am quite confident that the code is correct, however, one should check
whether the dealloc argument is correct. If it is wrong, there will be either
unnecessarily a if(var != NULL) { free } or the program will leak memory.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
[Bug fortran/43178] Pointless resetting to NULL for local ALLOCATABLEs
--- Comment #2 from burnus at gcc dot gnu dot org 2010-02-26 07:44 ---
Created an attachment (id=19966)
-- (http://gcc.gnu.org/bugzilla/attachment.cgi?id=19966action=view)
Improved patch
This patch changes:
a) There was a bug, which causes that non-SAVEd variables in procedures where
not re-initialized if they were (TREE_)STATIC due to -fmax-stack-var-size=1. I
now check also for is_main_program. (wrong-code)
b) Initializing the static variables in such cases is pointless if one needs to
initialize them explicitly afterwards. (missed optimization - will at least
increase the file size.)
(Note: I build the patch, I have not regtested it, but I think it is very
unlikely that it fails. What I wrote before still holds: The value for the
actual argument to the dummy dealloc should be checked - esp. in
trans-openmp.f90.)
Example: Have a look at the dump of. You should not see
static struct t1 x = {.a={-43, -43, ...
static struct t1 y = {.a={-43, -43, ...
but you should see
t1.2.a[S.3 + -1] = -43;
y.b.data = 0B;
t2.0.a[S.1 + -1] = -43;
! { dg-options -fmax-stack-var-size=1 }
module m
type t1
integer :: a(200) = -43
end type t1
type t2
integer :: a(200) = -43
integer, allocatable :: b(:)
end type t2
contains
subroutine sub()
type(t1) :: x
type(t2) :: y
if (x%a(1) /= -43) call abort()
if (y%a(1) /= -43) call abort()
!if (any (x%a /= -43)) call abort()
!if (any (y%a /= -43)) call abort()
end subroutine sub
end module m
use m
call sub()
call sub()
end
--
burnus at gcc dot gnu dot org changed:
What|Removed |Added
Attachment #19962|0 |1
is obsolete||
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43178
