Hi Richard,
On 14/10/16 23:53, Richard Biener wrote:
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 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 <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.
>From 8349bce90dcf2b17b6f129149b2073e7acc6fa8c Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org>
Date: Wed, 12 Oct 2016 13:48:07 +1100
Subject: [PATCH 1/3] Add-nonnull-to-pointer-from-VRP
---
gcc/testsuite/gcc.dg/torture/pr39074-2.c | 2 +-
gcc/testsuite/gcc.dg/torture/pr39074.c | 2 +-
gcc/tree-ssa-alias.h | 2 +-
gcc/tree-ssa-ccp.c | 2 +-
gcc/tree-ssa-structalias.c | 12 +++++++--
gcc/tree-ssanames.c | 29 +++++++++++++++++++++
gcc/tree-ssanames.h | 2 ++
gcc/tree-vrp.c | 44 +++++++++++++++++++++-----------
8 files changed, 74 insertions(+), 21 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/torture/pr39074-2.c b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
index 740b463..0693f2d 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074-2.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074-2.c
@@ -31,4 +31,4 @@ int main()
}
/* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr39074.c b/gcc/testsuite/gcc.dg/torture/pr39074.c
index 31ed499..54c444e 100644
--- a/gcc/testsuite/gcc.dg/torture/pr39074.c
+++ b/gcc/testsuite/gcc.dg/torture/pr39074.c
@@ -30,4 +30,4 @@ int main()
}
/* { dg-final { scan-tree-dump "y.._. = { i }" "alias" } } */
-/* { dg-final { scan-tree-dump "y.._., points-to vars: { D..... }" "alias" } } */
+/* { dg-final { scan-tree-dump "y.._., points-to NULL, points-to vars: { D..... }" "alias" } } */
diff --git a/gcc/tree-ssa-alias.h b/gcc/tree-ssa-alias.h
index 6680cc0..27a06fc 100644
--- a/gcc/tree-ssa-alias.h
+++ b/gcc/tree-ssa-alias.h
@@ -146,7 +146,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_singleton_or_null_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 *);
diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c
index fe9a313..61754d8 100644
--- a/gcc/tree-ssa-ccp.c
+++ b/gcc/tree-ssa-ccp.c
@@ -2135,7 +2135,7 @@ fold_builtin_alloca_with_align (gimple *stmt)
{
bool singleton_p;
unsigned uid;
- singleton_p = pt_solution_singleton_p (&pi->pt, &uid);
+ singleton_p = pt_solution_singleton_or_null_p (&pi->pt, &uid);
gcc_assert (singleton_p);
SET_DECL_PT_UID (var, uid);
}
diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c
index 2a4ab2f..78f4533 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -6451,6 +6451,7 @@ find_what_p_points_to (tree fndecl, tree p)
struct ptr_info_def *pi;
tree lookup_p = p;
varinfo_t vi;
+ bool nonnull = get_ptr_nonnull (p);
/* For parameters, get at the points-to set for the actual parm
decl. */
@@ -6466,6 +6467,12 @@ find_what_p_points_to (tree fndecl, tree p)
pi = get_ptr_info (p);
pi->pt = find_what_var_points_to (fndecl, vi);
+ /* Conservatively set to NULL from PTA (to true). */
+ pi->pt.null = 1;
+ /* Preserve pointer nonnull computed by VRP. See get_ptr_nonnull
+ in gcc/tree-ssaname.c for more information. */
+ if (nonnull)
+ set_ptr_nonnull (p);
}
@@ -6505,6 +6512,7 @@ pt_solution_reset (struct pt_solution *pt)
{
memset (pt, 0, sizeof (struct pt_solution));
pt->anything = true;
+ pt->null = true;
}
/* Set the points-to solution *PT to point only to the variables
@@ -6599,10 +6607,10 @@ pt_solution_empty_p (struct pt_solution *pt)
return the var uid in *UID. */
bool
-pt_solution_singleton_p (struct pt_solution *pt, unsigned *uid)
+pt_solution_singleton_or_null_p (struct pt_solution *pt, unsigned *uid)
{
if (pt->anything || pt->nonlocal || pt->escaped || pt->ipa_escaped
- || pt->null || pt->vars == NULL
+ || pt->vars == NULL
|| !bitmap_single_bit_set_p (pt->vars))
return false;
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 64ab13a..913d142 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,35 @@ get_range_info (const_tree name, wide_int *min, wide_int *max)
return SSA_NAME_RANGE_TYPE (name);
}
+/* Set nonnull attribute to pointer NAME. */
+
+void
+set_ptr_nonnull (tree name)
+{
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (name)));
+ struct ptr_info_def *pi = get_ptr_info (name);
+ pi->pt.null = 0;
+}
+
+/* Return nonnull attribute of pointer NAME. */
+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 true in PTA
+ 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.
+
+ When PTA analysis is improved, pt.anything, pt.nonlocal
+ and pt.escaped may also has to be considered before
+ deciding that pointer cannot point to NULL. */
+ return !pi->pt.null;
+}
+
/* Change non-zero bits bitmask of NAME. */
void
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index 4496e1d..d39cc9d 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -88,6 +88,8 @@ extern void set_ptr_info_alignment (struct ptr_info_def *, unsigned int,
extern void adjust_ptr_info_misalignment (struct ptr_info_def *,
unsigned int);
extern struct ptr_info_def *get_ptr_info (tree);
+extern void set_ptr_nonnull (tree);
+extern bool get_ptr_nonnull (const_tree);
extern tree copy_ssa_name_fn (struct function *, tree, gimple *);
extern void duplicate_ssa_name_ptr_info (tree, struct ptr_info_def *);
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 8a129c6..7e4f947 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -10603,18 +10603,24 @@ vrp_finalize (bool warn_array_bounds_p)
{
tree name = ssa_name (i);
- if (!name
- || POINTER_TYPE_P (TREE_TYPE (name))
- || (vr_value[i]->type == VR_VARYING)
- || (vr_value[i]->type == VR_UNDEFINED))
- continue;
+ if (!name
+ || (vr_value[i]->type == VR_VARYING)
+ || (vr_value[i]->type == VR_UNDEFINED)
+ || (TREE_CODE (vr_value[i]->min) != INTEGER_CST)
+ || (TREE_CODE (vr_value[i]->max) != INTEGER_CST))
+ continue;
- if ((TREE_CODE (vr_value[i]->min) == INTEGER_CST)
- && (TREE_CODE (vr_value[i]->max) == INTEGER_CST)
- && (vr_value[i]->type == VR_RANGE
- || vr_value[i]->type == VR_ANTI_RANGE))
- set_range_info (name, vr_value[i]->type, vr_value[i]->min,
- vr_value[i]->max);
+ if (POINTER_TYPE_P (TREE_TYPE (name))
+ && ((vr_value[i]->type == VR_RANGE
+ && range_includes_zero_p (vr_value[i]->min,
+ vr_value[i]->max) == 0)
+ || (vr_value[i]->type == VR_ANTI_RANGE
+ && range_includes_zero_p (vr_value[i]->min,
+ vr_value[i]->max) == 1)))
+ set_ptr_nonnull (name);
+ else if (!POINTER_TYPE_P (TREE_TYPE (name)))
+ set_range_info (name, vr_value[i]->type, vr_value[i]->min,
+ vr_value[i]->max);
}
substitute_and_fold (op_with_constant_singleton_value_range,
@@ -10817,17 +10823,25 @@ evrp_dom_walker::before_dom_children (basic_block bb)
def_operand_p def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);
/* Set the SSA with the value range. */
if (def_p
- && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME
- && INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p))))
+ && TREE_CODE (DEF_FROM_PTR (def_p)) == SSA_NAME)
{
tree def = DEF_FROM_PTR (def_p);
value_range *vr = get_value_range (def);
- if ((vr->type == VR_RANGE
- || vr->type == VR_ANTI_RANGE)
+ if (INTEGRAL_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+ && (vr->type == VR_RANGE
+ || vr->type == VR_ANTI_RANGE)
&& (TREE_CODE (vr->min) == INTEGER_CST)
&& (TREE_CODE (vr->max) == INTEGER_CST))
set_range_info (def, vr->type, vr->min, vr->max);
+ else if (POINTER_TYPE_P (TREE_TYPE (DEF_FROM_PTR (def_p)))
+ && ((vr->type == VR_RANGE
+ && range_includes_zero_p (vr->min,
+ vr->max) == 0)
+ || (vr->type == VR_ANTI_RANGE
+ && range_includes_zero_p (vr->min,
+ vr->max) == 1)))
+ set_ptr_nonnull (def);
}
}
else
--
2.7.4
