Patchwork [fortran] PR 54833 Don't wrap calls to free(a) in if (a != NULL)

login
register
mail settings
Submitter Thomas Koenig
Date Oct. 6, 2012, 11:22 a.m.
Message ID <50701465.3040104@netcologne.de>
Download mbox | patch
Permalink /patch/189680/
State New
Headers show

Comments

Thomas Koenig - Oct. 6, 2012, 11:22 a.m.
I wrote:

> the attached patch removes wrapping calls to free(a) by
> if (a != NULL) for some cases.  It is not complete, because
> automatic deallocation of allocatable structure components
> is not yet covered.

I accidentally posted an old version, which had a bug in
coarrays (basically was just missing an "else").  Regression-tested.

OK for trunk?

	Thomas

2012-10-06  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54833
         * trans.c (gfc_call_free):  Do not wrap the
         call to __builtin_free in check for NULL.
         (gfc_deallocate_with_status): For automatic deallocation without
         status for non-coarrays, don't wrap call to __builtin_free in
         check for NULL.

2012-10-06  Thomas König  <tkoenig@gcc.gnu.org>

         PR fortran/54833
         * gfortran.dg/auto_dealloc_3.f90:  New test.
Andreas Schwab - Oct. 27, 2012, 3:49 p.m.
Thomas Koenig <tkoenig@netcologne.de> writes:

>         PR fortran/54833
>         * trans.c (gfc_call_free):  Do not wrap the
>         call to __builtin_free in check for NULL.
>         (gfc_deallocate_with_status): For automatic deallocation without
>         status for non-coarrays, don't wrap call to __builtin_free in
>         check for NULL.

spawn /home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../gfortran -B/home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../ -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/ /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90 -fno-diagnostics-show-caret -Os -pedantic-errors -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -lm -o ./alloc_comp_assign_1.exe
/home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90: In function 'main':
/home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90:57:0: internal compiler error: in cselib_record_set, at cselib.c:2379
0x1029ac87 cselib_record_set
	../../gcc/gcc/cselib.c:2379
0x1029ac87 cselib_record_sets
	../../gcc/gcc/cselib.c:2596
0x1029b417 cselib_process_insn(rtx_def*)
	../../gcc/gcc/cselib.c:2649
0x104ec9ab reload_cse_regs_1
	../../gcc/gcc/postreload.c:224
0x104ee1db reload_cse_regs
	../../gcc/gcc/postreload.c:70
0x104ee1db rest_of_handle_postreload
	../../gcc/gcc/postreload.c:2289
0x104ee1db rest_of_handle_postreload
	../../gcc/gcc/postreload.c:2283

Andreas.
Thomas Koenig - Oct. 27, 2012, 5:02 p.m.
Hi Andreas,

> Thomas Koenig <tkoenig@netcologne.de> writes:
>
>>          PR fortran/54833
>>          * trans.c (gfc_call_free):  Do not wrap the
>>          call to __builtin_free in check for NULL.
>>          (gfc_deallocate_with_status): For automatic deallocation without
>>          status for non-coarrays, don't wrap call to __builtin_free in
>>          check for NULL.
>
> spawn /home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../gfortran -B/home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../ -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/ /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90 -fno-diagnostics-show-caret -Os -pedantic-errors -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -lm -o ./alloc_comp_assign_1.exe
> /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90: In function 'main':
> /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90:57:0: internal compiler error: in cselib_record_set, at cselib.c:2379
> 0x1029ac87 cselib_record_set
> 	../../gcc/gcc/cselib.c:2379

Strange, this looks more like a bug that is exposed through the patch.
I also don't see how such a change to the Fortran front end can
result in something invalid in postreload.

Does not happen on the architectures that I used, and I do not
have access to that architecture.

Does a (rougly) equivalent C case compile?

	Thomas
Andreas Schwab - Oct. 27, 2012, 5:41 p.m.
Thomas Koenig <tkoenig@netcologne.de> writes:

> Does not happen on the architectures that I used, and I do not
> have access to that architecture.

A cross compiler is enough.

> Does a (rougly) equivalent C case compile?

How does a (rougly) equivalent C case look like?

Andreas.
Steven Bosscher - Oct. 27, 2012, 5:48 p.m.
On Sat, Oct 27, 2012 at 7:41 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> How does a (rougly) equivalent C case look like?

Usually something can be constructed from the -fdump-tree-gimple dump.

Ciao!
Steven
Andreas Schwab - Oct. 27, 2012, 6:52 p.m.
Steven Bosscher <stevenb.gcc@gmail.com> writes:

> On Sat, Oct 27, 2012 at 7:41 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> How does a (rougly) equivalent C case look like?
>
> Usually something can be constructed from the -fdump-tree-gimple dump.

Doesn't look like.

Andreas.

Patch

Index: trans.c
===================================================================
--- trans.c	(Revision 191857)
+++ trans.c	(Arbeitskopie)
@@ -814,26 +814,23 @@  gfc_allocate_allocatable (stmtblock_t * block, tre
 }
 
 
-/* Free a given variable, if it's not NULL.  */
+/* Free a given variable.  If it is NULL, free takes care of this
+   automatically.  */
 tree
 gfc_call_free (tree var)
 {
   stmtblock_t block;
-  tree tmp, cond, call;
+  tree call;
 
   if (TREE_TYPE (var) != TREE_TYPE (pvoid_type_node))
     var = fold_convert (pvoid_type_node, var);
 
   gfc_start_block (&block);
   var = gfc_evaluate_now (var, &block);
-  cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, var,
-			  build_int_cst (pvoid_type_node, 0));
   call = build_call_expr_loc (input_location,
 			      builtin_decl_explicit (BUILT_IN_FREE),
 			      1, var);
-  tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond, call,
-			 build_empty_stmt (input_location));
-  gfc_add_expr_to_block (&block, tmp);
+  gfc_add_expr_to_block (&block, call);
 
   return gfc_finish_block (&block);
 }
@@ -861,11 +858,10 @@  gfc_call_free (tree var)
 	}
     }
 
-   In this front-end version, status doesn't have to be GFC_INTEGER_4.
-   Moreover, if CAN_FAIL is true, then we will not emit a runtime error,
-   even when no status variable is passed to us (this is used for
-   unconditional deallocation generated by the front-end at end of
-   each procedure).
+   In this front-end version, status doesn't have to be GFC_INTEGER_4.  If
+   CAN_FAIL is true, no status variable is passed and we are not dealing with
+   a coarray, we will simply call free().  This is used for unconditional
+   deallocation generated by the front-end at end of each procedure.
    
    If a runtime-message is possible, `expr' must point to the original
    expression being deallocated for its locus and variable name.
@@ -890,6 +886,14 @@  gfc_deallocate_with_status (tree pointer, tree sta
       STRIP_NOPS (pointer);
     }
 
+  else if (can_fail && status == NULL_TREE)
+    {
+      tmp = build_call_expr_loc (input_location,
+				 builtin_decl_explicit (BUILT_IN_FREE), 1,
+				 fold_convert (pvoid_type_node, pointer));
+      return tmp;
+    }
+
   cond = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node, pointer,
 			  build_int_cst (TREE_TYPE (pointer), 0));