[PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[Bin Cheng]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Hi,
GCC avoids multi-pointers/dangling-pointers of struct iv by allocating
multiple copies of the structure.  This patch is an obvious fix to the issue
by managing iv structures in obstack.

Bootstrap on x86_64, will apply to trunk if no objection.

Thanks,
bin

2015-06-26  Bin Cheng  <bin.cheng@arm.com>

	* tree-ssa-loop-ivopts.c (struct ivopts_data): New field iv_obstack.
	(tree_ssa_iv_optimize_init): Initialize iv_obstack.
	(alloc_iv): New parameter.  Allocate struct iv using obstack_alloc.
	(set_iv, find_interesting_uses_address, add_candidate_1): New
	argument.
	(find_interesting_uses_op): Don't duplicate struct iv.
	(free_loop_data): Don't free iv structure explicitly.
	(tree_ssa_iv_optimize_finalize): Free iv_obstack.

[-- Attachment #2: allocate-iv-using-obstack.txt --]
[-- Type: text/plain, Size: 4112 bytes --]

Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c	(revision 224930)
+++ gcc/tree-ssa-loop-ivopts.c	(working copy)
@@ -352,6 +352,9 @@ struct ivopts_data
   /* The maximum invariant id.  */
   unsigned max_inv_id;
 
+  /* Obstack for iv structure.  */
+  struct obstack iv_obstack;
+
   /* Whether to consider just related and important candidates when replacing a
      use.  */
   bool consider_all_candidates;
@@ -912,6 +915,7 @@ tree_ssa_iv_optimize_init (struct ivopts_data *dat
   data->inv_expr_id = 0;
   data->name_expansion_cache = NULL;
   decl_rtl_to_reset.create (20);
+  gcc_obstack_init (&data->iv_obstack);
 }
 
 /* Returns a memory object to that EXPR points.  In case we are able to
@@ -995,10 +999,12 @@ contain_complex_addr_expr (tree expr)
    for loop LOOP.  NO_OVERFLOW implies the iv doesn't overflow.  */
 
 static struct iv *
-alloc_iv (tree base, tree step, bool no_overflow = false)
+alloc_iv (struct ivopts_data *data, tree base, tree step,
+	  bool no_overflow = false)
 {
   tree expr = base;
-  struct iv *iv = XCNEW (struct iv);
+  struct iv *iv = (struct iv*) obstack_alloc (&data->iv_obstack,
+					      sizeof (struct iv));
   gcc_assert (step != NULL_TREE);
 
   /* Lower address expression in base except ones with DECL_P as operand.
@@ -1039,7 +1045,7 @@ set_iv (struct ivopts_data *data, tree iv, tree ba
   gcc_assert (!info->iv);
 
   bitmap_set_bit (data->relevant, SSA_NAME_VERSION (iv));
-  info->iv = alloc_iv (base, step, no_overflow);
+  info->iv = alloc_iv (data, base, step, no_overflow);
   info->iv->ssa_name = iv;
 }
 
@@ -1430,7 +1432,6 @@ static struct iv_use *
 find_interesting_uses_op (struct ivopts_data *data, tree op)
 {
   struct iv *iv;
-  struct iv *civ;
   gimple stmt;
   struct iv_use *use;
 
@@ -1456,14 +1457,11 @@ find_interesting_uses_op (struct ivopts_data *data
     }
   iv->have_use_for = true;
 
-  civ = XNEW (struct iv);
-  *civ = *iv;
-
   stmt = SSA_NAME_DEF_STMT (op);
   gcc_assert (gimple_code (stmt) == GIMPLE_PHI
 	      || is_gimple_assign (stmt));
 
-  use = record_use (data, NULL, civ, stmt, USE_NONLINEAR_EXPR);
+  use = record_use (data, NULL, iv, stmt, USE_NONLINEAR_EXPR);
   iv->use_id = use->id;
 
   return use;
@@ -2038,7 +2036,7 @@ find_interesting_uses_address (struct ivopts_data
 	}
     }
 
-  civ = alloc_iv (base, step);
+  civ = alloc_iv (data, base, step);
   record_group_use (data, op_p, civ, stmt, USE_ADDRESS);
   return;
 
@@ -2681,7 +2679,7 @@ add_candidate_1 (struct ivopts_data *data,
       if (!base && !step)
 	cand->iv = NULL;
       else
-	cand->iv = alloc_iv (base, step);
+	cand->iv = alloc_iv (data, base, step);
 
       cand->pos = pos;
       if (pos != IP_ORIGINAL && cand->iv)
@@ -7188,7 +7186,6 @@ free_loop_data (struct ivopts_data *data)
       struct version_info *info;
 
       info = ver_info (data, i);
-      free (info->iv);
       info->iv = NULL;
       info->has_nonlin_use = false;
       info->preserve_biv = false;
@@ -7207,13 +7204,11 @@ free_loop_data (struct ivopts_data *data)
 	  gcc_assert (sub->related_cands == NULL);
 	  gcc_assert (sub->n_map_members == 0 && sub->cost_map == NULL);
 
-	  free (sub->iv);
 	  pre = sub;
 	  sub = sub->next;
 	  free (pre);
 	}
 
-      free (use->iv);
       BITMAP_FREE (use->related_cands);
       for (j = 0; j < use->n_map_members; j++)
 	if (use->cost_map[j].depends_on)
@@ -7227,7 +7222,6 @@ free_loop_data (struct ivopts_data *data)
     {
       struct iv_cand *cand = iv_cand (data, i);
 
-      free (cand->iv);
       if (cand->depends_on)
 	BITMAP_FREE (cand->depends_on);
       free (cand);
@@ -7269,6 +7263,7 @@ tree_ssa_iv_optimize_finalize (struct ivopts_data
   delete data->inv_expr_tab;
   data->inv_expr_tab = NULL;
   free_affine_expand_cache (&data->name_expansion_cache);
+  obstack_free (&data->iv_obstack, NULL);
 }
 
 /* Returns true if the loop body BODY includes any function calls.  */

Re: [PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[Jeff Law]

On 06/26/2015 03:02 AM, Bin Cheng wrote:
> Hi,
> GCC avoids multi-pointers/dangling-pointers of struct iv by allocating
> multiple copies of the structure.  This patch is an obvious fix to the issue
> by managing iv structures in obstack.
>
> Bootstrap on x86_64, will apply to trunk if no objection.
>
> Thanks,
> bin
>
> 2015-06-26  Bin Cheng  <bin.cheng@arm.com>
>
> 	* tree-ssa-loop-ivopts.c (struct ivopts_data): New field iv_obstack.
> 	(tree_ssa_iv_optimize_init): Initialize iv_obstack.
> 	(alloc_iv): New parameter.  Allocate struct iv using obstack_alloc.
> 	(set_iv, find_interesting_uses_address, add_candidate_1): New
> 	argument.
> 	(find_interesting_uses_op): Don't duplicate struct iv.
> 	(free_loop_data): Don't free iv structure explicitly.
> 	(tree_ssa_iv_optimize_finalize): Free iv_obstack.
Presumably you're trying to simplify the memory management  here so that 
you don't have to track lifetimes of the IV structures so carefully, 
which in turn simplifies some upcoming patch?

Note we don't have a "no objection" policy for this kind of patch. 
However, I think it may make sense to look into having you as a 
maintainer for the IV optimizations if you're interested.

Jeff

Re: [PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[Bin.Cheng]

On Sat, Jun 27, 2015 at 5:13 AM, Jeff Law <law@redhat.com> wrote:
> On 06/26/2015 03:02 AM, Bin Cheng wrote:
>>
>> Hi,
>> GCC avoids multi-pointers/dangling-pointers of struct iv by allocating
>> multiple copies of the structure.  This patch is an obvious fix to the
>> issue
>> by managing iv structures in obstack.
>>
>> Bootstrap on x86_64, will apply to trunk if no objection.
>>
>> Thanks,
>> bin
>>
>> 2015-06-26  Bin Cheng  <bin.cheng@arm.com>
>>
>>         * tree-ssa-loop-ivopts.c (struct ivopts_data): New field
>> iv_obstack.
>>         (tree_ssa_iv_optimize_init): Initialize iv_obstack.
>>         (alloc_iv): New parameter.  Allocate struct iv using
>> obstack_alloc.
>>         (set_iv, find_interesting_uses_address, add_candidate_1): New
>>         argument.
>>         (find_interesting_uses_op): Don't duplicate struct iv.
>>         (free_loop_data): Don't free iv structure explicitly.
>>         (tree_ssa_iv_optimize_finalize): Free iv_obstack.
>
> Presumably you're trying to simplify the memory management  here so that you
> don't have to track lifetimes of the IV structures so carefully, which in
> turn simplifies some upcoming patch?
Yes, that's exactly the reason.  I am still on the way fixing
missed-optimizations in IVO, and plan to do some
refactoring/simplification afterwards.
>
> Note we don't have a "no objection" policy for this kind of patch. However,
> I think it may make sense to look into having you as a maintainer for the IV
> optimizations if you're interested.
Oh, that would be my great honor.

Thanks,
bin
>
> Jeff
>

Re: [PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[Richard Biener]

On Tue, Jun 30, 2015 at 4:31 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Sat, Jun 27, 2015 at 5:13 AM, Jeff Law <law@redhat.com> wrote:
>> On 06/26/2015 03:02 AM, Bin Cheng wrote:
>>>
>>> Hi,
>>> GCC avoids multi-pointers/dangling-pointers of struct iv by allocating
>>> multiple copies of the structure.  This patch is an obvious fix to the
>>> issue
>>> by managing iv structures in obstack.
>>>
>>> Bootstrap on x86_64, will apply to trunk if no objection.
>>>
>>> Thanks,
>>> bin
>>>
>>> 2015-06-26  Bin Cheng  <bin.cheng@arm.com>
>>>
>>>         * tree-ssa-loop-ivopts.c (struct ivopts_data): New field
>>> iv_obstack.
>>>         (tree_ssa_iv_optimize_init): Initialize iv_obstack.
>>>         (alloc_iv): New parameter.  Allocate struct iv using
>>> obstack_alloc.
>>>         (set_iv, find_interesting_uses_address, add_candidate_1): New
>>>         argument.
>>>         (find_interesting_uses_op): Don't duplicate struct iv.
>>>         (free_loop_data): Don't free iv structure explicitly.
>>>         (tree_ssa_iv_optimize_finalize): Free iv_obstack.
>>
>> Presumably you're trying to simplify the memory management  here so that you
>> don't have to track lifetimes of the IV structures so carefully, which in
>> turn simplifies some upcoming patch?
> Yes, that's exactly the reason.  I am still on the way fixing
> missed-optimizations in IVO, and plan to do some
> refactoring/simplification afterwards.
>>
>> Note we don't have a "no objection" policy for this kind of patch. However,
>> I think it may make sense to look into having you as a maintainer for the IV
>> optimizations if you're interested.
> Oh, that would be my great honor.

I'd support that.  Bin has done high quality work on IVOPTs in the past and he
knows when to ask questions (not that there ever were simple answers
to those...).

Thanks,
Richard.

> Thanks,
> bin
>>
>> Jeff
>>

Re: [PATCH GCC][refacor]Manage allocation of struct iv in obstack.

[Richard Biener]

On Tue, Jun 30, 2015 at 9:49 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Jun 30, 2015 at 4:31 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Sat, Jun 27, 2015 at 5:13 AM, Jeff Law <law@redhat.com> wrote:
>>> On 06/26/2015 03:02 AM, Bin Cheng wrote:
>>>>
>>>> Hi,
>>>> GCC avoids multi-pointers/dangling-pointers of struct iv by allocating
>>>> multiple copies of the structure.  This patch is an obvious fix to the
>>>> issue
>>>> by managing iv structures in obstack.
>>>>
>>>> Bootstrap on x86_64, will apply to trunk if no objection.
>>>>
>>>> Thanks,
>>>> bin
>>>>
>>>> 2015-06-26  Bin Cheng  <bin.cheng@arm.com>
>>>>
>>>>         * tree-ssa-loop-ivopts.c (struct ivopts_data): New field
>>>> iv_obstack.
>>>>         (tree_ssa_iv_optimize_init): Initialize iv_obstack.
>>>>         (alloc_iv): New parameter.  Allocate struct iv using
>>>> obstack_alloc.
>>>>         (set_iv, find_interesting_uses_address, add_candidate_1): New
>>>>         argument.
>>>>         (find_interesting_uses_op): Don't duplicate struct iv.
>>>>         (free_loop_data): Don't free iv structure explicitly.
>>>>         (tree_ssa_iv_optimize_finalize): Free iv_obstack.
>>>
>>> Presumably you're trying to simplify the memory management  here so that you
>>> don't have to track lifetimes of the IV structures so carefully, which in
>>> turn simplifies some upcoming patch?
>> Yes, that's exactly the reason.  I am still on the way fixing
>> missed-optimizations in IVO, and plan to do some
>> refactoring/simplification afterwards.
>>>
>>> Note we don't have a "no objection" policy for this kind of patch. However,
>>> I think it may make sense to look into having you as a maintainer for the IV
>>> optimizations if you're interested.
>> Oh, that would be my great honor.
>
> I'd support that.  Bin has done high quality work on IVOPTs in the past and he
> knows when to ask questions (not that there ever were simple answers
> to those...).

The patch is ok btw.

Thanks,
Richard.

> Thanks,
> Richard.
>
>> Thanks,
>> bin
>>>
>>> Jeff
>>>