Re: [graphite] Fix PR37485

["Diego Novillo" <dnovillo at google dot com>]

On Tue, Sep 16, 2008 at 18:41, Harsha Jagasia <harsha.jagasia@amd.com> wrote:

> Index: testsuite/gcc.dg/graphite/block-2.c
> ===================================================================
> --- testsuite/gcc.dg/graphite/block-2.c (revision 0)
> +++ testsuite/gcc.dg/graphite/block-2.c (revision 0)
> @@ -0,0 +1,33 @@
> +/* { dg-options "-O2 -floop-block -fdump-tree-graphite-all" } */
> +/* PR37485 fixed, but the test case triggers one more bug,
> +   which will be reported seperately under bugzilla.  */

Do you have the new bugzilla PR?  Otherwise, this comment will
make little sense in a few months.

> +/* Converts a GMP constant value to a tree and returns it.  */
> +
> +static tree
> +gmp_cst_to_tree (Value v)

s/a GMP constant value/GMP constant V/

> +/* Returns true if stack entry is a constant.  */
> +
> +static bool
> +iv_stack_entry_is_constant (iv_stack_entry *entry)

ENTRY needs documenting.

> +{
> +  return entry->kind == iv_stack_entry_const;
> +}
> +
> +/* Returns true if stack entry is a constant.  */
> +
> +static bool
> +iv_stack_entry_is_iv (iv_stack_entry *entry)

Likewise.

>  static void
> -loop_iv_stack_push (loop_iv_stack stack, tree iv, const char *name)
> +loop_iv_stack_push_iv (loop_iv_stack stack, tree iv, const char *name)
>  {
> +  iv_stack_entry* entry = XNEW (iv_stack_entry);

s/iv_stack_entry*/iv_stack_entry */

> +static void
> +loop_iv_stack_insert_constant (loop_iv_stack stack, int index,
> +                              tree constant)
> +{
> +  iv_stack_entry* entry = XNEW (iv_stack_entry);

Likewise.

> +static void
> +loop_iv_stack_remove_constants (loop_iv_stack stack)
> +{
> +  int i;
> +  iv_stack_entry* entry;

s/iv_stack_entry*/iv_stack_entry */

> +
> +  for (i = 0; VEC_iterate (iv_stack_entry_p, *stack, i, entry);)
> +    {
> +      if (iv_stack_entry_is_constant (entry))
> +       VEC_ordered_remove (iv_stack_entry_p, *stack, i);
> +      else
> +       i++;
> +    }

This is leaking memory.  You should free the object removed by
VEC_ordered_remove.

> @@ -3353,8 +3447,11 @@
>       mark_virtual_ops_in_bb (bb);
>       next_e = make_edge (bb,
>                          context_loop ? context_loop->latch : EXIT_BLOCK_PTR,
> -                         EDGE_FALLTHRU);;
> +                         EDGE_FALLTHRU);
> +      loop_iv_stack_patch_for_consts (ivstack,
> +                                     (struct clast_user_stmt *) stmt);
>       graphite_rename_ivs (gbb, scop, old_loop_father, ivstack);
> +      loop_iv_stack_remove_constants (ivstack);

We are leaking memory through ivstack.  It's being allocated in
gloog() but never released (VEC_ordered_remove in
loop_iv_stack_remove_constants does not free the removed object).
The individual elements from ivstack don't seem to be released
anywhere either.  Perhaps loop_iv_stack_pop should do this?


> @@ -4711,20 +4813,35 @@
>   VEC (scop_p, heap) *new_scops = VEC_alloc (scop_p, heap, 3);
>   int i;
>   scop_p scop;
> +  struct pointer_set_t* entry_exit_ptrs = pointer_set_create ();

s/pointer_set_t* entry_exit_ptrs/pointer_set_t *entry_exit_ptrs/

A call to pointer_set_destroy() is needed to free up the memory
used by this set.

> +typedef iv_stack_entry* iv_stack_entry_p;

s/iv_stack_entry*/iv_stack_entry */

OK with those changes.


Diego.