| Message ID | 4E29C76C.1070409@gmail.com |
|---|---|
| State | New |
| Headers | show |
Ooops... error in my ChangeLog: "fix" -> "free" 2011-07-22 Daniel Carrera <dcarrera@gmail.com> * trans.c (gfc__allocate_allocatable): [PR 4755] Do not free and the reallocate a variable that is already allocated. On 07/22/2011 08:54 PM, Daniel Carrera wrote: > Hello, > > This is a simple patch that fixes PR 4755. Currently the ALLOCATE > statement will free and re-allocate an already allocated scalar. The > Fortran standard says that this is an error. The attached patch fixes > the problem. > > I am also attaching two tree dumps of the same program, compiled before > and after the application of this patch. The test program is this: > > program test > integer, allocatable :: A(:), B[:] > integer :: stat > character(len=33) :: str > allocate(A(1), B[*], stat=stat) > end program > > > If you compare the "before" and "after" files you'll see that with this > patch GCC no longer tries to free and reallocate A(1). You will also > notice that the block for B[*] didn't change. The block for B[*] is > already correct in trunk and doesn't need changing. > > Here is my ChangeLog: > > > 2011-07-22 Daniel Carrera <dcarrera@gmail.com> > > * trans.c (gfc__allocate_allocatable): [PR 4755] Do not fix > and the reallocate a variable that is already allocated.
Hi Daniel, Daniel Carrera: > This is a simple patch that fixes PR 4755. It's PR 49755 - the 9 is missing in the number ... > 2011-07-22 Daniel Carrera <dcarrera@gmail.com> > > * trans.c (gfc__allocate_allocatable): [PR 4755] Do not fix > and the reallocate a variable that is already allocated. Instead of "[PR 4755]" use: <tab>PR <component>/<number>. Namely: > 2011-07-22 Daniel Carrera <dcarrera@gmail.com> > > PR fortran/49755 > * trans.c (gfc__allocate_allocatable): [PR 4755] Do not free > and the reallocate a variable that is already allocated. Please also include the test case from the PR in the patch - with added "! { dg-do run }" and also containing "! PR fortran/49755". However, the bug is not fully fixed. If you try the testcase in the bugreport, you will see that it still fails - the stat= is correctly set, but the array shape is wrong. The problem is that the "a.dim" setting should be done after allocation in the inner loop - also the a.offset is misplaced. If possible, one should also move the "D.1585" into the inner "else" block, which would save one from using the following code in case the memory is already allocated: D.1585 = a.data; ... a.data = D.1585; The promised dump, cf. testcase in the PR. a.dim[0].lbound = 1; a.dim[0].ubound = 5; a.dim[0].stride = 1; a.dim[1].lbound = 1; a.dim[1].ubound = 5; a.dim[1].stride = 5; overflow.4 = 0; if ((logical(kind=4)) __builtin_expect ((integer(kind=8)) (overflow.4 != 0), 0)) stat.3 = 5014; else { void * restrict D.1585; if ((logical(kind=4)) __builtin_expect ((integer(kind=8)) (a.data != 0B), 0)) { D.1585 = a.data; stat.3 = 5014; } else .... a.data = D.1585; } } a.offset = -6; if (stat.3 != 0) goto L.3; L.3: Tobias
Hi Tobias, Just a quick update so you know what I'm doing. On 07/22/2011 10:09 PM, Tobias Burnus wrote: > However, the bug is not fully fixed. If you try the testcase in the > bugreport, you will see that it still fails - the stat= is correctly > set, but the array shape is wrong. Working on it... > The problem is that the "a.dim" setting should be done after allocation > in the inner loop - also the a.offset is misplaced. If possible, one > should also move the "D.1585" into the inner "else" block, which would > save one from using the following code in case the memory is already > allocated: > > D.1585 = a.data; > ... > a.data = D.1585; I'm working on this right now. I see a few instances of code like this in the output tree. I think there is an opportunity to make the output tree much cleaner and maybe also make the GCC internals cleaner. I hope to have something to show you soon-ish. Daniel.
diff -r c8b6eb02738a gcc/fortran/trans.c --- a/gcc/fortran/trans.c Fri Jul 22 09:21:49 2011 +0000 +++ b/gcc/fortran/trans.c Fri Jul 22 20:36:23 2011 +0200 @@ -775,13 +775,8 @@ gfc_allocate_allocatable (stmtblock_t * stmtblock_t set_status_block; gfc_start_block (&set_status_block); - tmp = build_call_expr_loc (input_location, - built_in_decls[BUILT_IN_FREE], 1, - fold_convert (pvoid_type_node, mem)); - gfc_add_expr_to_block (&set_status_block, tmp); - tmp = gfc_allocate_using_malloc (&set_status_block, size, status); - gfc_add_modify (&set_status_block, res, fold_convert (type, tmp)); + gfc_add_modify (&set_status_block, res, fold_convert (type, mem)); gfc_add_modify (&set_status_block, status, build_int_cst (status_type, LIBERROR_ALLOCATION));
Hello, This is a simple patch that fixes PR 4755. Currently the ALLOCATE statement will free and re-allocate an already allocated scalar. The Fortran standard says that this is an error. The attached patch fixes the problem. I am also attaching two tree dumps of the same program, compiled before and after the application of this patch. The test program is this: program test integer, allocatable :: A(:), B[:] integer :: stat character(len=33) :: str allocate(A(1), B[*], stat=stat) end program If you compare the "before" and "after" files you'll see that with this patch GCC no longer tries to free and reallocate A(1). You will also notice that the block for B[*] didn't change. The block for B[*] is already correct in trunk and doesn't need changing. Here is my ChangeLog: 2011-07-22 Daniel Carrera <dcarrera@gmail.com> * trans.c (gfc__allocate_allocatable): [PR 4755] Do not fix and the reallocate a variable that is already allocated.