On 10 November 2011 21:31, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> This patch fixes some compiler memory leaks in SLP.
> For vect_free_oprnd_info I've removed the FREE_DEF_STMTS argument
> and am freeing the defs always, but set them to NULL when moving the vectors
> over elsewhere, because otherwise if vect_create_new_slp_node
> or vect_build_slp_tree fails after succeeding for a couple of iterations,
> we'd leak the rest or double free them.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Ira
>
> 2011-11-10 Jakub Jelinek <ja...@redhat.com>
>
> * tree-vect-slp.c (vect_free_slp_tree): Also free SLP_TREE_CHILDREN
> vector.
> (vect_create_new_slp_node): Don't allocate node before checking stmt
> type.
> (vect_free_oprnd_info): Remove FREE_DEF_STMTS argument, always
> free def_stmts vectors and additionally free oprnd_info.
> (vect_build_slp_tree): Adjust callers. Call it even if
> stop_recursion. If vect_create_new_slp_node or
> vect_build_slp_tree fails, properly handle freeing memory.
> If it succeeded, clear def_stmts in oprnd_info.
>
> --- gcc/tree-vect-slp.c.jj 2011-11-08 23:35:12.000000000 +0100
> +++ gcc/tree-vect-slp.c 2011-11-10 16:17:33.583105311 +0100
> @@ -75,8 +75,9 @@ vect_free_slp_tree (slp_tree node)
> return;
>
> FOR_EACH_VEC_ELT (slp_void_p, SLP_TREE_CHILDREN (node), i, child)
> - vect_free_slp_tree ((slp_tree)child);
> + vect_free_slp_tree ((slp_tree) child);
>
> + VEC_free (slp_void_p, heap, SLP_TREE_CHILDREN (node));
> VEC_free (gimple, heap, SLP_TREE_SCALAR_STMTS (node));
>
> if (SLP_TREE_VEC_STMTS (node))
> @@ -102,7 +103,7 @@ vect_free_slp_instance (slp_instance ins
> static slp_tree
> vect_create_new_slp_node (VEC (gimple, heap) *scalar_stmts)
> {
> - slp_tree node = XNEW (struct _slp_tree);
> + slp_tree node;
> gimple stmt = VEC_index (gimple, scalar_stmts, 0);
> unsigned int nops;
>
> @@ -117,6 +118,7 @@ vect_create_new_slp_node (VEC (gimple, h
> else
> return NULL;
>
> + node = XNEW (struct _slp_tree);
> SLP_TREE_SCALAR_STMTS (node) = scalar_stmts;
> SLP_TREE_VEC_STMTS (node) = NULL;
> SLP_TREE_CHILDREN (node) = VEC_alloc (slp_void_p, heap, nops);
> @@ -152,21 +154,19 @@ vect_create_oprnd_info (int nops, int gr
> }
>
>
> -/* Free operands info. Free def-stmts in FREE_DEF_STMTS is true.
> - (FREE_DEF_STMTS is true when the SLP analysis fails, and false when it
> - succeds. In the later case we don't need the operands info that we used
> to
> - check isomorphism of the stmts, but we still need the def-stmts - they are
> - used as scalar stmts in SLP nodes. */
> +/* Free operands info. */
> +
> static void
> -vect_free_oprnd_info (VEC (slp_oprnd_info, heap) **oprnds_info,
> - bool free_def_stmts)
> +vect_free_oprnd_info (VEC (slp_oprnd_info, heap) **oprnds_info)
> {
> int i;
> slp_oprnd_info oprnd_info;
>
> - if (free_def_stmts)
> - FOR_EACH_VEC_ELT (slp_oprnd_info, *oprnds_info, i, oprnd_info)
> + FOR_EACH_VEC_ELT (slp_oprnd_info, *oprnds_info, i, oprnd_info)
> + {
> VEC_free (gimple, heap, oprnd_info->def_stmts);
> + XDELETE (oprnd_info);
> + }
>
> VEC_free (slp_oprnd_info, heap, *oprnds_info);
> }
> @@ -502,7 +502,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -516,7 +516,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -532,7 +532,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -546,7 +546,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_generic_expr (vect_dump, scalar_type, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -576,7 +576,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -611,7 +611,7 @@ vect_build_slp_tree (loop_vec_info loop_
> {
> if (vect_print_dump_info (REPORT_SLP))
> fprintf (vect_dump, "Build SLP failed: no optab.");
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> icode = (int) optab_handler (optab, vec_mode);
> @@ -620,7 +620,7 @@ vect_build_slp_tree (loop_vec_info loop_
> if (vect_print_dump_info (REPORT_SLP))
> fprintf (vect_dump, "Build SLP failed: "
> "op not supported by target.");
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> optab_op2_mode = insn_data[icode].operand[2].mode;
> @@ -657,7 +657,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -671,7 +671,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -691,7 +691,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -707,7 +707,7 @@ vect_build_slp_tree (loop_vec_info loop_
> stmt, ncopies_for_cost,
> (i == 0), &oprnds_info))
> {
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -727,7 +727,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -744,7 +744,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -765,7 +765,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -785,7 +785,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -821,7 +821,7 @@ vect_build_slp_tree (loop_vec_info loop_
> }
>
> /* FORNOW: Not strided loads are not supported. */
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -838,7 +838,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> @@ -857,7 +857,7 @@ vect_build_slp_tree (loop_vec_info loop_
> print_gimple_stmt (vect_dump, stmt, 0, TDF_SLIM);
> }
>
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -867,7 +867,7 @@ vect_build_slp_tree (loop_vec_info loop_
> ncopies_for_cost, (i == 0),
> &oprnds_info))
> {
> - vect_free_oprnd_info (&oprnds_info, true);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
> }
> @@ -898,6 +898,7 @@ vect_build_slp_tree (loop_vec_info loop_
> *loads_permuted = true;
> }
>
> + vect_free_oprnd_info (&oprnds_info);
> return true;
> }
>
> @@ -916,15 +917,18 @@ vect_build_slp_tree (loop_vec_info loop_
> max_nunits, load_permutation, loads,
> vectorization_factor, loads_permuted))
> {
> - free (child);
> - vect_free_oprnd_info (&oprnds_info, true);
> + if (child)
> + oprnd_info->def_stmts = NULL;
> + vect_free_slp_tree (child);
> + vect_free_oprnd_info (&oprnds_info);
> return false;
> }
>
> + oprnd_info->def_stmts = NULL;
> VEC_quick_push (slp_void_p, SLP_TREE_CHILDREN (*node), child);
> }
>
> - vect_free_oprnd_info (&oprnds_info, false);
> + vect_free_oprnd_info (&oprnds_info);
> return true;
> }
>
>
> Jakub
>
