Message ID | 1a1dae3f-814d-e284-7aa9-9d2a59b9cbcc@redhat.com |
---|---|
State | New |
Headers | show |
Series | VRP: convert pointers of known quantity better | expand |
On 9/14/18 4:31 AM, Aldy Hernandez wrote: > Apparently, my work on VRP will never finish. There's an infinity of > things that can be tweaked ;-). > > First, we shouldn't drop to null/non-null when we know what the actual > pointer value is. For example, [1, 3] which we get when we store magic > numbers in a pointer (p == (char *)1). > > BTW, for this bit, I would much rather change range_int_cst_p to allow > VR_ANTI_RANGE, instead of inlining as I've done. Is there a reason > range_int_cst_p doesn't handle anti ranges? > > Also, [&foo, &foo] is known to be non-null. Don't drop to varying. > > OK? > > curr.patch > > commit 7f42e101d5d26ea866b739692858289a8dff4396 > Author: Aldy Hernandez <aldyh@redhat.com> > Date: Fri Sep 14 00:11:34 2018 +0200 > > * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of > known quantity. > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c > index 622ccbc2df7..22e5ee3c729 100644 > --- a/gcc/tree-vrp.c > +++ b/gcc/tree-vrp.c > @@ -1842,9 +1842,14 @@ extract_range_from_unary_expr (value_range *vr, > tree inner_type = op0_type; > tree outer_type = type; > > - /* If the expression evaluates to a pointer, we are only interested in > - determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]). */ > - if (POINTER_TYPE_P (type)) > + /* If the expression evaluates to a pointer of unknown quantity, > + we are only interested in determining if it evaluates to NULL > + [0, 0] or non-NULL (~[0, 0]). */ > + if (POINTER_TYPE_P (type) > + && !((vr0.type == VR_RANGE > + || vr0.type == VR_ANTI_RANGE) > + && TREE_CODE (vr0.min) == INTEGER_CST > + && TREE_CODE (vr0.max) == INTEGER_CST)) > { > if (!range_includes_zero_p (&vr0)) > set_value_range_to_nonnull (vr, type); I think this part is fine. Though I question how much time we want to spend dealing some clowns that do things like store integers into pointer objects :-) > @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr, > return; > } > > + /* If we have a non-constant range that we know is non-zero (for > + example [&foo, &foo] or [&foo, +MAX]), make it known, so we > + don't drop to VR_VARYING later. */ > + if (POINTER_TYPE_P (op0_type) > + && vr0.type == VR_RANGE > + && (TREE_CODE (vr0.min) != INTEGER_CST > + || TREE_CODE (vr0.max) != INTEGER_CST) > + && !range_includes_zero_p (&vr0)) > + set_value_range_to_nonnull (&vr0, op0_type); > + I don't think this is correct in the presence of weak symbols. I think you can query maybe_nonzero_address on the min/max and if both return > 0, then you know the result is non-null. Jeff
On 09/14/2018 05:25 PM, Jeff Law wrote: > On 9/14/18 4:31 AM, Aldy Hernandez wrote: >> Apparently, my work on VRP will never finish. There's an infinity of >> things that can be tweaked ;-). >> >> First, we shouldn't drop to null/non-null when we know what the actual >> pointer value is. For example, [1, 3] which we get when we store magic >> numbers in a pointer (p == (char *)1). >> >> BTW, for this bit, I would much rather change range_int_cst_p to allow >> VR_ANTI_RANGE, instead of inlining as I've done. Is there a reason >> range_int_cst_p doesn't handle anti ranges? >> >> Also, [&foo, &foo] is known to be non-null. Don't drop to varying. >> >> OK? >> >> curr.patch >> >> commit 7f42e101d5d26ea866b739692858289a8dff4396 >> Author: Aldy Hernandez <aldyh@redhat.com> >> Date: Fri Sep 14 00:11:34 2018 +0200 >> >> * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of >> known quantity. >> >> diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c >> index 622ccbc2df7..22e5ee3c729 100644 >> --- a/gcc/tree-vrp.c >> +++ b/gcc/tree-vrp.c >> @@ -1842,9 +1842,14 @@ extract_range_from_unary_expr (value_range *vr, >> tree inner_type = op0_type; >> tree outer_type = type; >> >> - /* If the expression evaluates to a pointer, we are only interested in >> - determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]). */ >> - if (POINTER_TYPE_P (type)) >> + /* If the expression evaluates to a pointer of unknown quantity, >> + we are only interested in determining if it evaluates to NULL >> + [0, 0] or non-NULL (~[0, 0]). */ >> + if (POINTER_TYPE_P (type) >> + && !((vr0.type == VR_RANGE >> + || vr0.type == VR_ANTI_RANGE) >> + && TREE_CODE (vr0.min) == INTEGER_CST >> + && TREE_CODE (vr0.max) == INTEGER_CST)) >> { >> if (!range_includes_zero_p (&vr0)) >> set_value_range_to_nonnull (vr, type); > I think this part is fine. Though I question how much time we want to > spend dealing some clowns that do things like store integers into > pointer objects :-) Hmmm. You're right. I take it back. I don't want to further complicate things. I never quite liked clowns as a kid. I have another approach to cleaning up the pointer conversion code that I will post separately. > >> @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr, >> return; >> } >> >> + /* If we have a non-constant range that we know is non-zero (for >> + example [&foo, &foo] or [&foo, +MAX]), make it known, so we >> + don't drop to VR_VARYING later. */ >> + if (POINTER_TYPE_P (op0_type) >> + && vr0.type == VR_RANGE >> + && (TREE_CODE (vr0.min) != INTEGER_CST >> + || TREE_CODE (vr0.max) != INTEGER_CST) >> + && !range_includes_zero_p (&vr0)) >> + set_value_range_to_nonnull (&vr0, op0_type); >> + > I don't think this is correct in the presence of weak symbols. I think > you can query maybe_nonzero_address on the min/max and if both return > > 0, then you know the result is non-null. I'm taking it all back. Perhaps I'll revisit this and test with weak symbols later. Thanks, and sorry for the noise. Aldy
commit 7f42e101d5d26ea866b739692858289a8dff4396 Author: Aldy Hernandez <aldyh@redhat.com> Date: Fri Sep 14 00:11:34 2018 +0200 * tree-vrp.c (extract_range_from_unary_expr): Handle pointers of known quantity. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 622ccbc2df7..22e5ee3c729 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -1842,9 +1842,14 @@ extract_range_from_unary_expr (value_range *vr, tree inner_type = op0_type; tree outer_type = type; - /* If the expression evaluates to a pointer, we are only interested in - determining if it evaluates to NULL [0, 0] or non-NULL (~[0, 0]). */ - if (POINTER_TYPE_P (type)) + /* If the expression evaluates to a pointer of unknown quantity, + we are only interested in determining if it evaluates to NULL + [0, 0] or non-NULL (~[0, 0]). */ + if (POINTER_TYPE_P (type) + && !((vr0.type == VR_RANGE + || vr0.type == VR_ANTI_RANGE) + && TREE_CODE (vr0.min) == INTEGER_CST + && TREE_CODE (vr0.max) == INTEGER_CST)) { if (!range_includes_zero_p (&vr0)) set_value_range_to_nonnull (vr, type); @@ -1855,6 +1860,16 @@ extract_range_from_unary_expr (value_range *vr, return; } + /* If we have a non-constant range that we know is non-zero (for + example [&foo, &foo] or [&foo, +MAX]), make it known, so we + don't drop to VR_VARYING later. */ + if (POINTER_TYPE_P (op0_type) + && vr0.type == VR_RANGE + && (TREE_CODE (vr0.min) != INTEGER_CST + || TREE_CODE (vr0.max) != INTEGER_CST) + && !range_includes_zero_p (&vr0)) + set_value_range_to_nonnull (&vr0, op0_type); + /* We normalize everything to a VR_RANGE, but for constant anti-ranges we must handle them by leaving the final result as an anti range. This allows us to convert things like