Set nonnull attribute to ptr_info_def based on VRP

kugan kugan.vivekanandarajah@linaro.org
Mon Oct 17 08:19:00 GMT 2016


Hi Richard,

On 14/10/16 23:53, Richard Biener wrote:
> On Fri, Oct 14, 2016 at 1:12 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi Richard,
>>
>>
>> On 13/10/16 20:44, Richard Biener wrote:
>>>
>>> On Thu, Oct 13, 2016 at 6:49 AM, kugan
>>> <kugan.vivekanandarajah@linaro.org> wrote:
>>>>
>>>> Hi Richard,
>>>>
>>>>>
>>>>> what does this try to do?  Preserve info VRP computed across PTA?
>>>>>
>>>>> I think we didn't yet sort out the nonlocal/escaped vs. null handling
>>>>> properly
>>>>> (or how PTA should handle get_ptr_nonnull).  The way you are using it
>>>>> asks for pt.null to be orthogonal to nonlocal/escaped and thus having
>>>>> nonlocal or escaped would also require setting ptr.null in PTA.  It then
>>>>> would be also more canonical to set it for pt.anything as well.  Which
>>>>> means conservatively handling it would be equivalent to flipping its
>>>>> semantic and changing its name to pt.nonnull.
>>>>>
>>>>> That said, you seem to be simply "reserving" the bit for VRP, keeping it
>>>>> conservatively true when "not computed".  I guess I'm fine with this for
>>>>> now
>>>>> but it should be documented in the header file that way.
>>>>>
>>>>
>>>> Thanks for the comments.
>>>>
>>>> To summarize, currently I am not relying on PTA analysis at all. Just
>>>> saving
>>>> null from VRP (or rather nonnull) and preserving it across PTA. Primary
>>>> intention is to pass it for PARM_DECL SSA names (from ipa-vrp).
>>>>
>>>> In this case, using  pt.anything/nonlocal/escaped will only make the
>>>> result
>>>> more pessimistic.
>>>>
>>>> Ideally, we should improve pt.null within PTA but for now as you said, I
>>>> will document it.
>>>>
>>>> When we start using pt.null from PTA analysis, we would also have to take
>>>> into account pt.anything/nonlocal/escaped.
>>>>
>>>> Does that make sense?
>>>
>>>
>>> Yes.
>>
>>
>>
>> Here is the revised patch based on the review. I also had to adjust two
>> testcases since we set pt.null conservatively and dumps that too.
>
> Hmm, sorry for another request, but seeing the testcase change I rather
> want to expose the conservatism only at find_what_p_points_to time.
>
> Thus,
>
> @@ -6382,6 +6382,8 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
>
>    *slot = pt = XOBNEW (&final_solutions_obstack, struct pt_solution);
>    memset (pt, 0, sizeof (struct pt_solution));
> +  /* Conservatively set to NULL from PTA (to true). */
> +  pt->null = 1;
>
>    /* Translate artificial variables into SSA_NAME_PTR_INFO
>       attributes.  */
>
> remove this hunk
>
> @@ -6466,6 +6469,10 @@ find_what_p_points_to (tree fndecl, tree p)
>
>    pi = get_ptr_info (p);
>    pi->pt = find_what_var_points_to (fndecl, vi);
> +  /* Preserve pointer nonnull computed by VRP.  See get_ptr_nonnull
> +     in gcc/tree-ssaname.c for more information.  */
> +  if (nonnull)
> +    set_ptr_nonnull (p);
>
> and add pi->pt.null = true; here (with the comment from above).  This
> preserves the (possibly incorrect) PTA solution in the dumps but does
> not leak it to SSA_NAME_PTR_INFO.
>
> +bool
> +get_ptr_nonnull (const_tree name)
> +{
> +  gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
> +  struct ptr_info_def *pi = SSA_NAME_PTR_INFO (name);
> +  if (pi == NULL)
> +    return false;
> +  /* TODO Now pt->null is conservatively set to null in PTA
>
> set to true
>
> +     analysis. vrp is the only pass (including ipa-vrp)
> +     that clears pt.null via set_ptr_nonull when it knows
> +     for sure. PTA will preserves the pt.null value set by VRP.
> +
>
> Ok with those changes.

Thanks for the review. Here is the updated patch. I also had to add 
pt.null to true in pt_solution_reset.

I will commit this version if there is no objection.

Thanks,
Kugan

> Thanks,
> Richard.
>
>
>> Thanks,
>> Kugan
>>
>> gcc/ChangeLog:
>>
>> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>         * tree-ssa-alias.h (pt_solution_singleton_or_null_p): Renamed from
>>         pt_solution_singleton_p.
>>         * tree-ssa-ccp.c (fold_builtin_alloca_with_align): Use renamed
>>         pt_solution_singleton_or_null_p from pt_solution_singleton_p.
>>         * tree-ssa-structalias.c (find_what_var_points_to): Conservatively
>> set
>>         pt.null to 1.
>>         (find_what_p_points_to): Preserve pointer nonnull computed by VRP.
>>         (pt_solution_singleton_or_null_p): Renamed from
>>         pt_solution_singleton_p.
>>         * tree-ssanames.h (set_ptr_nonnull): Declare.
>>         (get_ptr_nonnull): Likewise.
>>         * tree-ssanames.c (set_ptr_nonnull): New.
>>         (get_ptr_nonnull): Likewise.
>>         * tree-vrp.c (vrp_finalize): Set ptr that are nonnull.
>>         (evrp_dom_walker::before_dom_children): Likewise.
>>
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2016-10-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>>
>>         * gcc.dg/torture/pr39074-2.c: Adjust testcase.
>>         * gcc.dg/torture/pr39074.c: Likewise.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-nonnull-to-pointer-from-VRP.patch
Type: text/x-patch
Size: 8777 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20161017/b6153cbe/attachment.bin>


More information about the Gcc-patches mailing list