[4.3 branch] Re: Memory leak in ifcvt.c

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

Richard Guenther wrote:
> On Mon, Jun 30, 2008 at 9:33 PM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>   
> > Updated as richard has requested.
> >
> > 2008-06-30  Kenneth Zadeck <zadeck@naturalbridge.com>
> >
> >   * df-scan.c (df_scan_free_ref_vec, df_scan_free_mws_vec): New
> >   macros.
> >          (df_scan_free_internal): Free data structures not
> >   allocated in storage pools.
> >   (df_mw_hardreg_chain_delete_eq_uses): Use df_scan_free_mws_vec.
> >   (df_refs_add_to_chains): Use df_scan_free_ref_vec and
> >   df_scan_free_mws_vec.
> >   * dse.c (dse_step6): Free offset_map_p and offset_map_n
> >   unconditionally.
> >
> > 2008-06-30  Kenneth Zadeck <zadeck@naturalbridge.com>
> >
> >   * ifcvt.c (cond_move_process_if_block): Free vectors on false
> >   return.
> >
> > Committed as revisions 137284 and 137285.
> >     
>
> Thanks.  This seems to have a very positive effect on some testcases,
> can you back-port these fixes to the 4.3 branch if applicable?
>
> Thanks,
> Richard.
>   
committed as revision 137319 to 4.3 branch after bootstrapping.

kenny


2008-07-01  Kenneth Zadeck <zadeck@naturalbridge.com>

   PR rtl-optimization/34744
   * df-scan.c (df_scan_free_ref_vec, df_scan_free_mws_vec): New
   macros.
          (df_scan_free_internal): Free data structures not
   allocated in storage pools.
   (df_mw_hardreg_chain_delete_eq_uses): Use df_scan_free_mws_vec.
   (df_refs_add_to_chains): Use df_scan_free_ref_vec and
   df_scan_free_mws_vec.
   * dse.c (dse_step6): Free offset_map_p and offset_map_n
   unconditionally.
   * ifcvt.c (cond_move_process_if_block): Free vectors on false
   return.


Index: gcc/df-scan.c
===================================================================
--- gcc/df-scan.c	(revision 137316)
+++ gcc/df-scan.c	(working copy)
@@ -60,6 +60,21 @@ along with GCC; see the file COPYING3.
 #define EPILOGUE_USES(REGNO)  0
 #endif
 
+/* The following two macros free the vecs that hold either the refs or
+   the mw refs.  They are a little tricky because the vec has 0
+   elements is special and is not to be freed.  */ 
+#define df_scan_free_ref_vec(V) \
+  do { \
+    if (V && *V) \
+      free (V);  \
+  } while (0)
+
+#define df_scan_free_mws_vec(V) \
+  do { \
+    if (V && *V) \
+      free (V);  \
+  } while (0)
+
 /* The bitmap_obstack is used to hold some static variables that
    should not be reset after each function is compiled.  */
 
@@ -170,11 +185,43 @@ struct df_scan_problem_data
 
 typedef struct df_scan_bb_info *df_scan_bb_info_t;
 
+
+/* Internal function to shut down the scanning problem.  */
 static void 
 df_scan_free_internal (void)
 {
   struct df_scan_problem_data *problem_data
     = (struct df_scan_problem_data *) df_scan->problem_data;
+  unsigned int i;
+  basic_block bb;
+
+  /* The vectors that hold the refs are not pool allocated because
+     they come in many sizes.  This makes them impossible to delete
+     all at once.  */
+  for (i = 0; i < DF_INSN_SIZE(); i++)
+    {
+      struct df_insn_info *insn_info = DF_INSN_UID_GET(i);
+      /* Skip the insns that have no insn_info or have been
+	 deleted.  */
+      if (insn_info)
+	{
+	  df_scan_free_ref_vec (insn_info->defs);
+	  df_scan_free_ref_vec (insn_info->uses);
+	  df_scan_free_ref_vec (insn_info->eq_uses);
+	  df_scan_free_mws_vec (insn_info->mw_hardregs);
+	}
+    }
+
+  FOR_ALL_BB (bb)
+    {
+      unsigned int bb_index = bb->index;
+      struct df_scan_bb_info *bb_info = df_scan_get_bb_info (bb_index);
+      if (bb_info)
+	{
+	  df_scan_free_ref_vec (bb_info->artificial_defs);
+	  df_scan_free_ref_vec (bb_info->artificial_uses);
+	}
+    }
 
   free (df->def_info.refs);
   free (df->def_info.begin);
@@ -1981,7 +2028,7 @@ df_mw_hardreg_chain_delete_eq_uses (stru
 
   if (count == 0)
     {
-      free (insn_info->mw_hardregs);
+      df_scan_free_mws_vec (insn_info->mw_hardregs);
       insn_info->mw_hardregs = df_null_mw_rec;
       return 0;
     }
@@ -2498,8 +2545,7 @@ df_refs_add_to_chains (struct df_collect
 	 chain specially.  */
       if (collection_rec->def_vec)
 	{
-	  if (insn_rec->defs && *insn_rec->defs)
-	    free (insn_rec->defs);
+	  df_scan_free_ref_vec (insn_rec->defs);
 	  insn_rec->defs 
 	    = df_install_refs (bb, collection_rec->def_vec, 
 			       collection_rec->next_def,
@@ -2508,8 +2554,7 @@ df_refs_add_to_chains (struct df_collect
 	}
       if (collection_rec->use_vec)
 	{
-	  if (insn_rec->uses && *insn_rec->uses)
-	    free (insn_rec->uses);
+	  df_scan_free_ref_vec (insn_rec->uses);
 	  insn_rec->uses 
 	    = df_install_refs (bb, collection_rec->use_vec, 
 			       collection_rec->next_use,
@@ -2518,8 +2563,7 @@ df_refs_add_to_chains (struct df_collect
 	}
       if (collection_rec->eq_use_vec)
 	{
-	  if (insn_rec->eq_uses && *insn_rec->eq_uses)
-	    free (insn_rec->eq_uses);
+	  df_scan_free_ref_vec (insn_rec->eq_uses);
 	  insn_rec->eq_uses 
 	    = df_install_refs (bb, collection_rec->eq_use_vec, 
 			       collection_rec->next_eq_use,
@@ -2528,8 +2572,7 @@ df_refs_add_to_chains (struct df_collect
 	}
       if (collection_rec->mw_vec)
 	{
-	  if (insn_rec->mw_hardregs && *insn_rec->mw_hardregs)
-	    free (insn_rec->mw_hardregs);
+	  df_scan_free_mws_vec (insn_rec->mw_hardregs);
 	  insn_rec->mw_hardregs 
 	    = df_install_mws (collection_rec->mw_vec, 
 			      collection_rec->next_mw);
@@ -2539,15 +2582,13 @@ df_refs_add_to_chains (struct df_collect
     {
       struct df_scan_bb_info *bb_info = df_scan_get_bb_info (bb->index);
 
-      if (bb_info->artificial_defs && *bb_info->artificial_defs)
-	free (bb_info->artificial_defs);
+      df_scan_free_ref_vec (bb_info->artificial_defs);
       bb_info->artificial_defs 
 	= df_install_refs (bb, collection_rec->def_vec, 
 			   collection_rec->next_def,
 			   df->def_regs,
 			   &df->def_info, false);
-      if (bb_info->artificial_uses && *bb_info->artificial_uses)
-	free (bb_info->artificial_uses);
+      df_scan_free_ref_vec (bb_info->artificial_uses);
       bb_info->artificial_uses 
 	= df_install_refs (bb, collection_rec->use_vec, 
 			   collection_rec->next_use,
Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c	(revision 137316)
+++ gcc/ifcvt.c	(working copy)
@@ -2606,7 +2606,11 @@ cond_move_process_if_block (struct noce_
   /* Make sure the blocks are suitable.  */
   if (!check_cond_move_block (then_bb, then_vals, then_regs, cond)
       || (else_bb && !check_cond_move_block (else_bb, else_vals, else_regs, cond)))
-    return FALSE;
+    {
+      VEC_free (int, heap, then_regs);
+      VEC_free (int, heap, else_regs);
+      return FALSE;
+    }
 
   /* Make sure the blocks can be used together.  If the same register
      is set in both blocks, and is not set to a constant in both
@@ -2627,7 +2631,11 @@ cond_move_process_if_block (struct noce_
 	  if (!CONSTANT_P (then_vals[reg])
 	      && !CONSTANT_P (else_vals[reg])
 	      && !rtx_equal_p (then_vals[reg], else_vals[reg]))
-	    return FALSE;
+	    {
+	      VEC_free (int, heap, then_regs);
+	      VEC_free (int, heap, else_regs);
+	      return FALSE;
+	    }
 	}
     }
 
@@ -2641,7 +2649,11 @@ cond_move_process_if_block (struct noce_
      branches, since if we convert we are going to always execute
      them.  */
   if (c > MAX_CONDITIONAL_EXECUTE)
-    return FALSE;
+    {
+      VEC_free (int, heap, then_regs);
+      VEC_free (int, heap, else_regs);
+      return FALSE;
+    }
 
   /* Try to emit the conditional moves.  First do the then block,
      then do anything left in the else blocks.  */
@@ -2653,11 +2665,17 @@ cond_move_process_if_block (struct noce_
 					  then_vals, else_vals, true)))
     {
       end_sequence ();
+      VEC_free (int, heap, then_regs);
+      VEC_free (int, heap, else_regs);
       return FALSE;
     }
   seq = end_ifcvt_sequence (if_info);
   if (!seq)
-    return FALSE;
+    {
+      VEC_free (int, heap, then_regs);
+      VEC_free (int, heap, else_regs);
+      return FALSE;
+    }
 
   loc_insn = first_active_insn (then_bb);
   if (!loc_insn)
@@ -2690,7 +2708,6 @@ cond_move_process_if_block (struct noce_
 
   VEC_free (int, heap, then_regs);
   VEC_free (int, heap, else_regs);
-
   return TRUE;
 }
 
Index: gcc/dse.c
===================================================================
--- gcc/dse.c	(revision 137316)
+++ gcc/dse.c	(working copy)
@@ -3163,43 +3163,30 @@ dse_step6 (bool global_done)
   group_info_t group;
   basic_block bb;
   
-  if (global_done)
-    {
-      for (i = 0; VEC_iterate (group_info_t, rtx_group_vec, i, group); i++)
-	{
-	  free (group->offset_map_n);
-	  free (group->offset_map_p);
-	  BITMAP_FREE (group->store1_n);
-	  BITMAP_FREE (group->store1_p);
-	  BITMAP_FREE (group->store2_n);
-	  BITMAP_FREE (group->store2_p);
-	  BITMAP_FREE (group->group_kill);
-	}
-
-      FOR_ALL_BB (bb)
-	{
-	  bb_info_t bb_info = bb_table[bb->index];
-	  BITMAP_FREE (bb_info->gen);
-	  if (bb_info->kill)
-	    BITMAP_FREE (bb_info->kill);
-	  if (bb_info->in)
-	    BITMAP_FREE (bb_info->in);
-	  if (bb_info->out)
-	    BITMAP_FREE (bb_info->out);
-	}
-    }
-  else
+  for (i = 0; VEC_iterate (group_info_t, rtx_group_vec, i, group); i++)
     {
-      for (i = 0; VEC_iterate (group_info_t, rtx_group_vec, i, group); i++)
-	{
-	  BITMAP_FREE (group->store1_n);
-	  BITMAP_FREE (group->store1_p);
-	  BITMAP_FREE (group->store2_n);
-	  BITMAP_FREE (group->store2_p);
-	  BITMAP_FREE (group->group_kill);
-	}
+      free (group->offset_map_n);
+      free (group->offset_map_p);
+      BITMAP_FREE (group->store1_n);
+      BITMAP_FREE (group->store1_p);
+      BITMAP_FREE (group->store2_n);
+      BITMAP_FREE (group->store2_p);
+      BITMAP_FREE (group->group_kill);
     }
 
+  if (global_done)
+    FOR_ALL_BB (bb)
+      {
+	bb_info_t bb_info = bb_table[bb->index];
+	BITMAP_FREE (bb_info->gen);
+	if (bb_info->kill)
+	  BITMAP_FREE (bb_info->kill);
+	if (bb_info->in)
+	  BITMAP_FREE (bb_info->in);
+	if (bb_info->out)
+	  BITMAP_FREE (bb_info->out);
+      }
+
   if (clear_alias_sets)
     {
       BITMAP_FREE (clear_alias_sets);
@@ -3224,7 +3211,6 @@ dse_step6 (bool global_done)
 }
 
 
-
 /* -------------------------------------------------------------------------
    DSE
    ------------------------------------------------------------------------- */