Hi Richard,
On 07/10/16 21:03, Richard Biener wrote:
On Fri, Oct 7, 2016 at 2:53 AM, kugan <kugan.vivekanandara...@linaro.org> wrote:
Hi Richard,
Thanks for the review.
On 09/08/16 18:58, Richard Biener wrote:
On Tue, Aug 9, 2016 at 12:58 AM, kugan
<kugan.vivekanandara...@linaro.org> wrote:
Hi Jakub,
Thanks for the review.
On 08/08/16 16:40, Jakub Jelinek wrote:
On Mon, Aug 08, 2016 at 01:36:51PM +1000, kugan wrote:
diff --git a/gcc/tree-ssanames.h b/gcc/tree-ssanames.h
index c81b1a1..6e34433 100644
--- a/gcc/tree-ssanames.h
+++ b/gcc/tree-ssanames.h
@@ -43,6 +43,9 @@ struct GTY(()) ptr_info_def
above alignment. Access only through the same helper functions
as
align
above. */
unsigned int misalign;
+ /* When this pointer is knonw to be nnonnull this would be true
otherwise
+ false. */
+ bool nonnull_p;
};
Why do you need this? Doesn't the pt.null bit represent that already?
It looks like I can use this. As in gcc/tree-ssa-alias.h:
/* Nonzero if the points-to set includes 'nothing', the points-to set
includes memory at address NULL. */
unsigned int null : 1;
But in gcc/tree-ssa-alias.c, ptrs_compare_unequal has the following
comment
which says:
/* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
but those require pt.null to be conservatively correct. */
Does that means it can be wrong at times? I haven't looked it in detail
yet
but if it is, it would be a problem.
Yes, this bit is not correctly computed for all cases (well, it works
for trivial
ones but for example misses weaks and maybe some other corner-cases).
Currently there are no consumers of this bit so the incorrectness
doesn't matter.
I suggest you simply use that bit but set it conservatively from PTA (to
true)
for now and adjust it from VRP only.
I do have some patches for fixinig some of the .nonnull_p issues in PTA
but
they come at a cost of more constraints.
Here is a version of the patch that does this. I have a follow up patch to
use this in ipa-vrp. I will send it for review once this is OK.
Bootstrapped and regression tested on x86_64-linux-gnu with no new
regressions. Is this OK for trunk?
@@ -6359,7 +6361,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
if (vi->is_artificial_var)
{
if (vi->id == nothing_id)
- pt->null = 1;
+ ;
please leave this here.
Done.
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
+ || pt->vars == NULL
|| !bitmap_single_bit_set_p (pt->vars))
return false;
humm... this is a correctness problem I guess. A latent one currently
depending on who relies on it.
There is a single use of the above predicate which looks for the
points-to solution of a __builtin_alloca call. We should somehow
arrange for its solution to not include ->null. Or, simpler, change
the predicates name to pt_solution_singleton_or_null_p () as
the use does not care for pt->null.
I have changed the 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;
= false;
+}
There is the question on how pt.null semantics should be
with respect to pt.anything, pt.escaped and pt.nonlocal.
I think get_ptr_nonnull needs to return false if any
of those are set.
I have added pt.anything.
I am assuming if pt.escaped, it cannot become null?
My intention is to eliminate for pt.nonlocal too. Like in:
static __attribute__((noinline, noclone))
int foo (int *p)
{
if (!p)
return 0;
*p = 1;
}
struct st
{
int a;
int b;
};
int bar (struct st *s)
{
if (!s)
return 0;
foo (&s->a);
foo (&s->b);
}
And also in find_what_p_points_to we can overwrite pt.null. So I have
changed that too.
Please find the updated patch attached. Does this look better?
Thanks,
Kugan
Richard.
Thanks,
Kugan
gcc/ChangeLog:
2016-10-07 Kugan Vivekanandarajah <kug...@linaro.org>
* tree-ssa-alias.c (dump_points_to_solution): Dump non null as it
is set to null conservatively.
* tree-ssa-structalias.c (find_what_var_points_to): Set to null
conservatively.
(pt_solution_singleton_p): Dont check null.
* 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.
Richard.
Also, formatting and spelling:
s/knonw/known/
s/nnon/non/
s/bool /bool /
I will change this.
Thanks,
Kugan
Jakub
>From a2c163ca9e7d3569cc2d58bc188fcd466dfe2ff7 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/tree-ssa-alias.c | 4 ++--
gcc/tree-ssa-alias.h | 2 +-
gcc/tree-ssa-ccp.c | 2 +-
gcc/tree-ssa-structalias.c | 13 ++++++++-----
gcc/tree-ssanames.c | 22 ++++++++++++++++++++++
gcc/tree-ssanames.h | 2 ++
gcc/tree-vrp.c | 44 +++++++++++++++++++++++++++++---------------
7 files changed, 65 insertions(+), 24 deletions(-)
diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 01bef17..dff11b1 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -515,8 +515,8 @@ dump_points_to_solution (FILE *file, struct pt_solution *pt)
if (pt->ipa_escaped)
fprintf (file, ", points-to unit escaped");
- if (pt->null)
- fprintf (file, ", points-to NULL");
+ if (!pt->null)
+ fprintf (file, ", points-to non NULL");
if (pt->vars)
{
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..d451ba2 100644
--- a/gcc/tree-ssa-structalias.c
+++ b/gcc/tree-ssa-structalias.c
@@ -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. */
@@ -6391,9 +6393,7 @@ find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
if (vi->is_artificial_var)
{
- if (vi->id == nothing_id)
- pt->null = 1;
- else if (vi->id == escaped_id)
+ if (vi->id == escaped_id)
{
if (in_ipa_mode)
pt->ipa_escaped = 1;
@@ -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,8 @@ find_what_p_points_to (tree fndecl, tree p)
pi = get_ptr_info (p);
pi->pt = find_what_var_points_to (fndecl, vi);
+ if (nonnull)
+ set_ptr_nonnull (p);
}
@@ -6599,10 +6602,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..1d2ec01 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -374,6 +374,28 @@ 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
+ || pi->pt.anything)
+ return false;
+ 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
