Message ID | 717fb095-f0a9-6980-ddbd-e755a4fd6457@linaro.org |
---|---|
State | New |
Headers | show |
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? Also, formatting and spelling: s/knonw/known/ s/nnon/non/ s/bool /bool / Jakub
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. > Also, formatting and spelling: > s/knonw/known/ > s/nnon/non/ > s/bool /bool / I will change this. Thanks, Kugan > > Jakub >
On Tue, Aug 9, 2016 at 12:58 AM, kugan <kugan.vivekanandarajah@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. Richard. >> Also, formatting and spelling: >> s/knonw/known/ >> s/nnon/non/ >> s/bool /bool / > > > I will change this. > > Thanks, > Kugan >> >> >> Jakub >> >
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c index 91a8f97..19c9027 100644 --- a/gcc/tree-ssanames.c +++ b/gcc/tree-ssanames.c @@ -374,6 +374,27 @@ 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, bool nonnull_p) +{ + gcc_assert (POINTER_TYPE_P (TREE_TYPE (name))); + struct ptr_info_def *pi = get_ptr_info (name); + pi->nonnull_p = nonnull_p;; +} + +/* Return nonnull attribute of pointer NAME. */ +bool +get_ptr_nonnull (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; + return pi->nonnull_p; +} + /* Change non-zero bits bitmask of NAME. */ void 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; }; /* Value range information for SSA_NAMEs representing non-pointer variables. */ @@ -89,6 +92,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 name, bool nonnull_p); +extern bool get_ptr_nonnull (tree name); 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 7c7ad91..40c4d48 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -10352,18 +10352,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, true); + 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,