diff mbox

[PR43513,1/3] Replace vla with array - Implementation.

Message ID 4E7EFF0F.4060503@mentor.com
State New
Headers show

Commit Message

Tom de Vries Sept. 25, 2011, 10:14 a.m. UTC 
On 09/25/2011 10:57 AM, Richard Guenther wrote:
> On Sat, Sep 24, 2011 at 5:29 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> This is an updated version of the patch. I have 2 new patches and an
>>> updated testcase which I will sent out individually.
>>>
>>> Patch set was bootstrapped and reg-tested on x86_64.
>>>
>>> Ok for trunk?
>>>
>>> Thanks,
>>> - Tom
>>>
>>> 2011-07-30  Tom de Vries  <tom@codesourcery.com>
>>>
>>>       PR middle-end/43513
>>>       * Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule.
>>>       * tree-ssa-ccp.c (params.h): Include.
>>>       (fold_builtin_alloca_for_var): New function.
>>>       (ccp_fold_stmt): Use fold_builtin_alloca_for_var.
>>
>> We have detected another fallout on some Ada code: the transformation replaces
>> a call to __builtin_alloca with &var, i.e. it introduces an aliased variable,
>> which invalidates the points-to information of some subsequent call, fooling
>> DSE into thinking that it can eliminate a live store.
> 
> Ugh, yeah.  I suppose PTA assigned a HEAP var as pointed-to object for the
> original pointer, even if the transformed stmt
> 
>  orig_ptr_1 = &a;
> 
> has the points-to information preserved for orig_ptr_1 further propagation of
> &a will make accesses through orig_ptr_1 have different alias properties.
> 
> What should work in this special case of a singleton points-to set of orig_ptr_1
> (might want to check that) is, do
> 
>   SET_DECL_PT_UID (decl-of-a, DECL_UID (pointed-to orig_ptr_1));
> 
> The brute force approach is not acceptable (it'll wreck IPA points-to info).
> 
> A helper like pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
> whould be nice to have for this.
> 
> Note that we don't have points-to information computed during the first
> CCP pass, so the above should be conditional on SSA_NAME_PTR_INFO
> being present and not ! ->anything (but then assert that we actually do have
> a singleton, or fail the folding).
> 

I tried to implement the approach you describe above in attached patch.
Currently testing on x86_64.

Thanks,
- Tom

> Richard.
> 
>> The brute force approach
>>
>> Index: tree-ssa-ccp.c
>> ===================================================================
>> --- tree-ssa-ccp.c      (revision 179038)
>> +++ tree-ssa-ccp.c      (working copy)
>> @@ -2014,7 +2014,10 @@ do_ssa_ccp (void)
>>   ccp_initialize ();
>>   ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node);
>>   if (ccp_finalize ())
>> -    return (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals);
>> +    return (TODO_cleanup_cfg
>> +           | TODO_update_ssa
>> +           | TODO_rebuild_alias
>> +           | TODO_remove_unused_locals);
>>   else
>>     return 0;
>>  }
>>
>> works, but we might want to be move clever.  Thoughts?
>>
>> --
>> Eric Botcazou
>>

Comments

Eric Botcazou Sept. 26, 2011, 10:11 a.m. UTC | #1 
> I tried to implement the approach you describe above in attached patch.

Thanks a lot, this indeed fixes the problem!

> Currently testing on x86_64.

Please also install the testcase I posted in the other message in conjunction 
with the fix.  Thanks in advance.
Richard Biener Sept. 26, 2011, 10:29 a.m. UTC | #2 
On Sun, 25 Sep 2011, Tom de Vries wrote:

> On 09/25/2011 10:57 AM, Richard Guenther wrote:
> > On Sat, Sep 24, 2011 at 5:29 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> >>> This is an updated version of the patch. I have 2 new patches and an
> >>> updated testcase which I will sent out individually.
> >>>
> >>> Patch set was bootstrapped and reg-tested on x86_64.
> >>>
> >>> Ok for trunk?
> >>>
> >>> Thanks,
> >>> - Tom
> >>>
> >>> 2011-07-30  Tom de Vries  <tom@codesourcery.com>
> >>>
> >>>       PR middle-end/43513
> >>>       * Makefile.in (tree-ssa-ccp.o): Add $(PARAMS_H) to rule.
> >>>       * tree-ssa-ccp.c (params.h): Include.
> >>>       (fold_builtin_alloca_for_var): New function.
> >>>       (ccp_fold_stmt): Use fold_builtin_alloca_for_var.
> >>
> >> We have detected another fallout on some Ada code: the transformation replaces
> >> a call to __builtin_alloca with &var, i.e. it introduces an aliased variable,
> >> which invalidates the points-to information of some subsequent call, fooling
> >> DSE into thinking that it can eliminate a live store.
> > 
> > Ugh, yeah.  I suppose PTA assigned a HEAP var as pointed-to object for the
> > original pointer, even if the transformed stmt
> > 
> >  orig_ptr_1 = &a;
> > 
> > has the points-to information preserved for orig_ptr_1 further propagation of
> > &a will make accesses through orig_ptr_1 have different alias properties.
> > 
> > What should work in this special case of a singleton points-to set of orig_ptr_1
> > (might want to check that) is, do
> > 
> >   SET_DECL_PT_UID (decl-of-a, DECL_UID (pointed-to orig_ptr_1));
> > 
> > The brute force approach is not acceptable (it'll wreck IPA points-to info).
> > 
> > A helper like pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
> > whould be nice to have for this.
> > 
> > Note that we don't have points-to information computed during the first
> > CCP pass, so the above should be conditional on SSA_NAME_PTR_INFO
> > being present and not ! ->anything (but then assert that we actually do have
> > a singleton, or fail the folding).
> > 
> 
> I tried to implement the approach you describe above in attached patch.
> Currently testing on x86_64.

Looks good to me (with a changelog entry).

Thanks,
Richard.

> Thanks,
> - Tom
> 
> > Richard.
> > 
> >> The brute force approach
> >>
> >> Index: tree-ssa-ccp.c
> >> ===================================================================
> >> --- tree-ssa-ccp.c      (revision 179038)
> >> +++ tree-ssa-ccp.c      (working copy)
> >> @@ -2014,7 +2014,10 @@ do_ssa_ccp (void)
> >>   ccp_initialize ();
> >>   ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node);
> >>   if (ccp_finalize ())
> >> -    return (TODO_cleanup_cfg | TODO_update_ssa | TODO_remove_unused_locals);
> >> +    return (TODO_cleanup_cfg
> >> +           | TODO_update_ssa
> >> +           | TODO_rebuild_alias
> >> +           | TODO_remove_unused_locals);
> >>   else
> >>     return 0;
> >>  }
> >>
> >> works, but we might want to be move clever.  Thoughts?
> >>
> >> --
> >> Eric Botcazou
> >>
> 
>
diff mbox

Patch

Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c (revision 179043)
+++ gcc/tree-ssa-ccp.c (working copy)
@@ -1729,6 +1729,17 @@  fold_builtin_alloca_for_var (gimple stmt
   array_type = build_array_type_nelts (elem_type, n_elem);
   var = create_tmp_var (array_type, NULL);
   DECL_ALIGN (var) = align;
+  {
+    struct ptr_info_def *pi = SSA_NAME_PTR_INFO (lhs);
+    if (pi != NULL && !pi->pt.anything)
+      {
+	bool singleton_p;
+	unsigned uid;
+	singleton_p = pt_solution_singleton_p (&pi->pt, &uid);
+	gcc_assert (singleton_p);
+	SET_DECL_PT_UID (var, uid);
+      }
+  }
 
   /* Fold alloca to the address of the array.  */
   return fold_convert (TREE_TYPE (lhs), build_fold_addr_expr (var));
Index: gcc/tree-ssa-alias.h
===================================================================
--- gcc/tree-ssa-alias.h (revision 179043)
+++ gcc/tree-ssa-alias.h (working copy)
@@ -126,6 +126,7 @@  extern void dump_alias_stats (FILE *);
 /* In tree-ssa-structalias.c  */
 extern unsigned int compute_may_aliases (void);
 extern bool pt_solution_empty_p (struct pt_solution *);
+extern bool pt_solution_singleton_p (struct pt_solution *, unsigned *);
 extern bool pt_solution_includes_global (struct pt_solution *);
 extern bool pt_solution_includes (struct pt_solution *, const_tree);
 extern bool pt_solutions_intersect (struct pt_solution *, struct pt_solution *);
Index: gcc/tree-ssa-structalias.c
===================================================================
--- gcc/tree-ssa-structalias.c (revision 179043)
+++ gcc/tree-ssa-structalias.c (working copy)
@@ -5978,6 +5978,21 @@  pt_solution_empty_p (struct pt_solution
   return true;
 }
 
+/* Return true if the points-to solution *PT only point to a single var, and
+   return the var uid in *UID.  */
+
+bool
+pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
+{
+  if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
+      || pt->null || pt->vars == NULL
+      || !bitmap_single_bit_set_p (pt->vars))
+    return false;
+
+  *uid = bitmap_first_set_bit (pt->vars);
+  return true;
+}
+
 /* Return true if the points-to solution *PT includes global memory.  */
 
 bool