On Fri, Oct 14, 2016 at 1:12 AM, kugan
<kugan.vivekanandara...@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.vivekanandara...@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,
Richard.
> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2016-10-14 Kugan Vivekanandarajah <kug...@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 <kug...@linaro.org>
>
> * gcc.dg/torture/pr39074-2.c: Adjust testcase.
> * gcc.dg/torture/pr39074.c: Likewise.
