This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix tree-data-ref DDR_LOOP_NEST handling (PR tree-optimization/29330)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Sebastian Pop <sebastian dot pop at cri dot ensmp dot fr>, Daniel Berlin <dberlin at dberlin dot org>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Fri, 6 Oct 2006 11:08:56 -0400
- Subject: [PATCH] Fix tree-data-ref DDR_LOOP_NEST handling (PR tree-optimization/29330)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
As vec.h documentation states, vectors at least when they use safe push
shouldn't be passed around using value, but using reference, because a safe
push might realloc them. This is what happens in the attached testcase,
compute_data_dependences_for_loop allocates initially a 3 entry vector
but as there are 4 nested loops, the vector is reallocated, but
compute_data_dependences_for_loop isn't aware of that and still
reuses the old pointer. This is fixed by the
find_loop_nest_1/find_loop_nest/compute_data_dependences_for_loop
hunks. When looking around the code, I also find that the loop_nest
vector is never freed, so we have a memory leak (it is a tiny bit tricky,
because all the loops share the same loop_nest vector), free_data_ref
could use a DR_FREE_ACCESS_FNS macro which does exactly what free_data_ref
was doing and the macro was unused, and lastly tree-loop-linear.c forgot
to free various data structures if it decided to bail after
compute_data_dependences_for_loop was called.
Tested on x86_64-linux.
Ok for trunk?
The tree-loop-linear.c change is also applicable to 4.1 branch (i.e.
we are leaking memory there too), is that part ok for 4.1 too?
2006-10-06 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/29330
* tree-data-ref.c (free_data_ref): Use DR_FREE_ACCESS_FNS macro.
(initialize_data_dependence_relation): Clear DDR_LOOP_NEST pointer
on newly allocated ddrs.
(find_loop_nest_1, find_loop_nest): Change LOOP_NEST to a pointer
to VEC (loop_p, heap) pointer.
(compute_data_dependences_for_loop): Adjust caller.
(free_dependence_relations): Free DDR_LOOP_NEST.
* tree-loop-linear.c (linear_transform_loops): Don't forget to
free DEPENDENCE_RELATIONS and DATAREFS.
* gcc.dg/pr29330.c: New test.
--- gcc/tree-data-ref.c.jj 2006-10-05 00:28:42.000000000 +0200
+++ gcc/tree-data-ref.c 2006-10-06 16:04:39.000000000 +0200
@@ -1913,11 +1913,7 @@ analyze_offset (tree offset, tree *invar
static void
free_data_ref (data_reference_p dr)
{
- if (DR_TYPE(dr) == ARRAY_REF_TYPE)
- VEC_free (tree, heap, dr->object_info.access_fns);
- else
- VEC_free (tree, heap, dr->first_location.access_fns);
-
+ DR_FREE_ACCESS_FNS (dr);
free (dr);
}
@@ -2171,6 +2167,7 @@ initialize_data_dependence_relation (str
res = XNEW (struct data_dependence_relation);
DDR_A (res) = a;
DDR_B (res) = b;
+ DDR_LOOP_NEST (res) = NULL;
if (a == NULL || b == NULL)
{
@@ -4205,7 +4202,7 @@ find_data_references_in_loop (struct loo
/* Recursive helper function. */
static bool
-find_loop_nest_1 (struct loop *loop, VEC (loop_p, heap) *loop_nest)
+find_loop_nest_1 (struct loop *loop, VEC (loop_p, heap) **loop_nest)
{
/* Inner loops of the nest should not contain siblings. Example:
when there are two consecutive loops,
@@ -4224,7 +4221,7 @@ find_loop_nest_1 (struct loop *loop, VEC
if (loop->next)
return false;
- VEC_safe_push (loop_p, heap, loop_nest, loop);
+ VEC_safe_push (loop_p, heap, *loop_nest, loop);
if (loop->inner)
return find_loop_nest_1 (loop->inner, loop_nest);
return true;
@@ -4236,9 +4233,9 @@ find_loop_nest_1 (struct loop *loop, VEC
appear in the classic distance vector. */
static bool
-find_loop_nest (struct loop *loop, VEC (loop_p, heap) *loop_nest)
+find_loop_nest (struct loop *loop, VEC (loop_p, heap) **loop_nest)
{
- VEC_safe_push (loop_p, heap, loop_nest, loop);
+ VEC_safe_push (loop_p, heap, *loop_nest, loop);
if (loop->inner)
return find_loop_nest_1 (loop->inner, loop_nest);
return true;
@@ -4265,7 +4262,7 @@ compute_data_dependences_for_loop (struc
is not computable, give up without spending time to compute other
dependences. */
if (!loop_nest
- || !find_loop_nest (loop_nest, vloops)
+ || !find_loop_nest (loop_nest, &vloops)
|| find_data_references_in_loop (loop, datarefs) == chrec_dont_know)
{
struct data_dependence_relation *ddr;
@@ -4435,10 +4432,22 @@ free_dependence_relations (VEC (ddr_p, h
{
unsigned int i;
struct data_dependence_relation *ddr;
+ VEC (loop_p, heap) *loop_nest = NULL;
for (i = 0; VEC_iterate (ddr_p, dependence_relations, i, ddr); i++)
- free_dependence_relation (ddr);
+ {
+ if (ddr == NULL)
+ continue;
+ if (loop_nest == NULL)
+ loop_nest = DDR_LOOP_NEST (ddr);
+ else
+ gcc_assert (DDR_LOOP_NEST (ddr) == NULL
+ || DDR_LOOP_NEST (ddr) == loop_nest);
+ free_dependence_relation (ddr);
+ }
+ if (loop_nest)
+ VEC_free (loop_p, heap, loop_nest);
VEC_free (ddr_p, heap, dependence_relations);
}
--- gcc/tree-loop-linear.c.jj 2006-10-05 12:02:47.000000000 +0200
+++ gcc/tree-loop-linear.c 2006-10-06 15:07:24.000000000 +0200
@@ -308,7 +308,7 @@ linear_transform_loops (struct loops *lo
{
if (dump_file)
fprintf (dump_file, "Won't transform loop. Optimal transform is the identity transform\n");
- continue;
+ goto free_and_continue;
}
/* Check whether the transformation is legal. */
@@ -316,15 +316,15 @@ linear_transform_loops (struct loops *lo
{
if (dump_file)
fprintf (dump_file, "Can't transform loop, transform is illegal:\n");
- continue;
+ goto free_and_continue;
}
before = gcc_loopnest_to_lambda_loopnest (loops, loop_nest, &oldivs,
&invariants);
if (!before)
- continue;
-
+ goto free_and_continue;
+
if (dump_file)
{
fprintf (dump_file, "Before:\n");
@@ -346,6 +346,7 @@ linear_transform_loops (struct loops *lo
if (dump_file)
fprintf (dump_file, "Successfully transformed loop.\n");
+ free_and_continue:
free_dependence_relations (dependence_relations);
free_data_refs (datarefs);
}
--- gcc/testsuite/gcc.dg/pr29330.c.jj 2006-10-06 15:13:23.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr29330.c 2006-10-06 15:12:55.000000000 +0200
@@ -0,0 +1,15 @@
+/* PR tree-optimization/29330 */
+/* { dg-do compile } */
+/* { dg-options "-O -ftree-loop-linear" } */
+
+int buf[2][2][2][2];
+
+void
+f (void)
+{
+ for (int a = 0; a < 2; ++a)
+ for (int b = 0; b < 2; ++b)
+ for (int c = 0; c < 2; ++c)
+ for (int d = 0; d < 2; ++d)
+ buf[a][b][c][d] = 0;
+}
Jakub