diff mbox

[RFC] Fix pointer diff (was: -fsanitize=pointer-overflow support (PR sanitizer/80998))

Message ID alpine.DEB.2.20.1707011757000.1896@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse July 1, 2017, 4:41 p.m. UTC
On Thu, 22 Jun 2017, Richard Biener wrote:

> On Thu, 22 Jun 2017, Marc Glisse wrote:
>
>> On Thu, 22 Jun 2017, Richard Biener wrote:
>>
>>>> If we consider pointers as unsigned, with a subtraction that has a signed
>>>> result with the constraint that overflow is undefined, we cannot model
>>>> that
>>>> optimally with just the usual signed/unsigned operations, so I am in favor
>>>> of
>>>> POINTER_DIFF, at least in the long run (together with having a signed
>>>> second
>>>> argument for POINTER_PLUS, etc). For 64-bit platforms it might have been
>>>> easier to declare that the upper half (3/4 ?) of the address space doesn't
>>>> exist...
>>>
>>> I repeatedly thought of POINTER_DIFF_EXPR but adding such a basic tree
>>> code is quite a big job.
>>
>> Yes :-(
>> It is probably not realistic to introduce it just to avoid a couple
>> regressions while fixing a bug.
>>
>>> So we'd have POINTER_DIFF_EXPR take two pointer typed args and produce
>>> ptrdiff_t.  What's the advantage of having this?
>>
>> It represents q-p with one statement instead of 3 (long)q-(long)p or 4
>> (long)((ulong)q-(ulong)p). It allows us to stay in the pointer world, so
>> (q-p)>0 is equivalent to p<q, not just (long)p<(long)q. It properly models
>> what (undefined) overflow means for pointers.
>>
>> Of course it is hard to know in advance if that's significant or
>> negligible, maybe size_t finds its way in too many places anyway.
>
> As with all those experiments ...
>
> Well, if I would sell this as a consultant to somebody I'd estimate
> 3 man months for this work which realistically means you have to
> start now otherwise you won't make it this stage 1.

I wrote a quick prototype to see what the fallout would look like.
Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
languages with no regression. Sure, it is probably not a complete
migration, there are likely a few places still converting to ptrdiff_t
to perform a regular subtraction, but this seems to indicate that the
work isn't as bad as using a signed type in pointer_plus_expr for
instance.

Comments

Richard Biener July 3, 2017, 12:37 p.m. UTC | #1
On Sat, 1 Jul 2017, Marc Glisse wrote:

> On Thu, 22 Jun 2017, Richard Biener wrote:
> 
> > On Thu, 22 Jun 2017, Marc Glisse wrote:
> > 
> > > On Thu, 22 Jun 2017, Richard Biener wrote:
> > > 
> > > > > If we consider pointers as unsigned, with a subtraction that has a
> > > > > signed
> > > > > result with the constraint that overflow is undefined, we cannot model
> > > > > that
> > > > > optimally with just the usual signed/unsigned operations, so I am in
> > > > > favor
> > > > > of
> > > > > POINTER_DIFF, at least in the long run (together with having a signed
> > > > > second
> > > > > argument for POINTER_PLUS, etc). For 64-bit platforms it might have
> > > > > been
> > > > > easier to declare that the upper half (3/4 ?) of the address space
> > > > > doesn't
> > > > > exist...
> > > > 
> > > > I repeatedly thought of POINTER_DIFF_EXPR but adding such a basic tree
> > > > code is quite a big job.
> > > 
> > > Yes :-(
> > > It is probably not realistic to introduce it just to avoid a couple
> > > regressions while fixing a bug.
> > > 
> > > > So we'd have POINTER_DIFF_EXPR take two pointer typed args and produce
> > > > ptrdiff_t.  What's the advantage of having this?
> > > 
> > > It represents q-p with one statement instead of 3 (long)q-(long)p or 4
> > > (long)((ulong)q-(ulong)p). It allows us to stay in the pointer world, so
> > > (q-p)>0 is equivalent to p<q, not just (long)p<(long)q. It properly models
> > > what (undefined) overflow means for pointers.
> > > 
> > > Of course it is hard to know in advance if that's significant or
> > > negligible, maybe size_t finds its way in too many places anyway.
> > 
> > As with all those experiments ...
> > 
> > Well, if I would sell this as a consultant to somebody I'd estimate
> > 3 man months for this work which realistically means you have to
> > start now otherwise you won't make it this stage 1.
> 
> I wrote a quick prototype to see what the fallout would look like.
> Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
> languages with no regression. Sure, it is probably not a complete
> migration, there are likely a few places still converting to ptrdiff_t
> to perform a regular subtraction, but this seems to indicate that the
> work isn't as bad as using a signed type in pointer_plus_expr for
> instance.

The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
from POINTER_MINUS_EXPR in some cases I guess).

The tree code needs documenting in tree.def and generic.texi.

Otherwise ok(*).

Thanks,
Richard.

(*) ok, just kidding -- or maybe not
Marc Glisse Oct. 9, 2017, 10:55 a.m. UTC | #2
On Mon, 3 Jul 2017, Richard Biener wrote:

> On Sat, 1 Jul 2017, Marc Glisse wrote:
>
>> I wrote a quick prototype to see what the fallout would look like.
>> Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
>> languages with no regression. Sure, it is probably not a complete
>> migration, there are likely a few places still converting to ptrdiff_t
>> to perform a regular subtraction, but this seems to indicate that the
>> work isn't as bad as using a signed type in pointer_plus_expr for
>> instance.
>
> The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
> from POINTER_MINUS_EXPR in some cases I guess).
>
> The tree code needs documenting in tree.def and generic.texi.
>
> Otherwise ok(*).
>
> Thanks,
> Richard.
>
> (*) ok, just kidding -- or maybe not

I updated the prototype a bit. Some things I noticed:

- the C front-end has support for address spaces that seems to imply that 
pointer subtraction (and division by the size) may be done in a type 
larger than ptrdiff_t. Currently, it generates 
(ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is some 
type potentially larger than ptrdiff_t. I am thus generating a 
POINTER_DIFF_EXPR with that type, while I was originally hoping its type 
would always be ptrdiff_t. It complicates code and I am not sure I didn't 
break address spaces anyway... (expansion likely needs to do the inttype 
dance)

Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t (what 
POINTER_PLUS_EXPR takes as second argument) always the same type 
signed/unsigned? Counter-examples: m32c (when !TARGET_A16), visium, 
darwin, powerpcspe, s390, vms... and it isn't even always the same that is 
bigger than the other. That's quite messy.

- I had to use @@ in match.pd, not for constants, but because in GENERIC 
we sometimes ended up with pointers where operand_equal_p said yes but 
types_match disagreed.

- It currently regresses 2 go tests: net/http runtime/debug
Richard Biener Oct. 19, 2017, 2:53 p.m. UTC | #3
On Mon, Oct 9, 2017 at 12:55 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Mon, 3 Jul 2017, Richard Biener wrote:
>
>> On Sat, 1 Jul 2017, Marc Glisse wrote:
>>
>>> I wrote a quick prototype to see what the fallout would look like.
>>> Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
>>> languages with no regression. Sure, it is probably not a complete
>>> migration, there are likely a few places still converting to ptrdiff_t
>>> to perform a regular subtraction, but this seems to indicate that the
>>> work isn't as bad as using a signed type in pointer_plus_expr for
>>> instance.
>>
>>
>> The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
>> from POINTER_MINUS_EXPR in some cases I guess).
>>
>> The tree code needs documenting in tree.def and generic.texi.
>>
>> Otherwise ok(*).
>>
>> Thanks,
>> Richard.
>>
>> (*) ok, just kidding -- or maybe not
>
>
> I updated the prototype a bit. Some things I noticed:
>
> - the C front-end has support for address spaces that seems to imply that
> pointer subtraction (and division by the size) may be done in a type larger
> than ptrdiff_t. Currently, it generates
> (ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is some type
> potentially larger than ptrdiff_t.

It uses a ptrdiff_t corresponding to the respective address space, yes.
That we use sizetype elsewhere unconditionally is a bug :/

 I am thus generating a POINTER_DIFF_EXPR
> with that type, while I was originally hoping its type would always be
> ptrdiff_t. It complicates code and I am not sure I didn't break address
> spaces anyway... (expansion likely needs to do the inttype dance)

I think that's fine.  Ideally targets would provide a type to use for each
respective address space given we have targets that have sizetype smaller
than ptr_mode.

> Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t (what
> POINTER_PLUS_EXPR takes as second argument) always the same type
> signed/unsigned?

POINTER_PLUS_EXPR takes 'sizetype', not size_t.  So the answer is clearly
no.  And yes, that's ugly and broken and should be fixed.

> Counter-examples: m32c (when !TARGET_A16), visium, darwin,
> powerpcspe, s390, vms... and it isn't even always the same that is bigger
> than the other. That's quite messy.

Eh.  Yeah, targets are free to choose 'sizetype' and they do so for
efficiency.  IMHO we should get rid of this "feature".

> - I had to use @@ in match.pd, not for constants, but because in GENERIC we
> sometimes ended up with pointers where operand_equal_p said yes but
> types_match disagreed.
>
> - It currently regresses 2 go tests: net/http runtime/debug

Those are flaky for me and fail sometimes and sometimes not.

+@item POINTER_DIFF_EXPR
+This node represents pointer subtraction.  The two operands always
+have pointer/reference type.  The second operand is always an unsigned
+integer type compatible with sizetype.  It returns a signed integer.

the 2nd sentence looks bogusly copied.

+      /* FIXME.  */
+      if (code == POINTER_DIFF_EXPR)
+       return int_const_binop (MINUS_EXPR,
+                               fold_convert (ptrdiff_type_node, arg1),
+                               fold_convert (ptrdiff_type_node, arg2));

  wide_int_to_tree (type, wi::to_widest (arg1) - wi::to_widest (arg2));

?

+    case POINTER_DIFF_EXPR:
+      {
+       if (!POINTER_TYPE_P (rhs1_type)
+           || !POINTER_TYPE_P (rhs2_type)
+           // || !useless_type_conversion_p (rhs2_type, rhs1_type)

types_compatible_p (rhs1_type, rhs2_type)?

+           // || !useless_type_conversion_p (ptrdiff_type_node, lhs_type))
+           || TREE_CODE (lhs_type) != INTEGER_TYPE
+           || TYPE_UNSIGNED (lhs_type))
+         {
+           error ("type mismatch in pointer diff expression");
+           debug_generic_stmt (lhs_type);
+           debug_generic_stmt (rhs1_type);
+           debug_generic_stmt (rhs2_type);
+           return true;

there's also verify_expr which could want adjustment for newly created
tree kinds.

So if the precision of the result is larger than that of the pointer operands
we sign-extend the result, right?  So the subtraction is performed in precision
of the pointer operands and then sign-extended/truncated to the result type?
Which means it is _not_ a widening subtraction to get around the
half-address-space
issue.  The tree.def documentation should reflect this semantic
detail.  Not sure
if the RTL expansion follows it.

I think that we'd ideally have a single legal INTEGER_TYPE precision
per pointer type precision and that those precisions should match...
we don't have to follow the mistakes of POINTER_PLUS_EXPR.

So ... above verify TYPE_PRECISION (rhs1_type) == TYPE_PRECISION (lhs_type)?
Some targets have 24bit ptr_mode but no 24bit integer type which means the
FE likely chooses 32bit int for the difference computation.  But the middle-end
should be free to create a 24bit precision type with SImode.

Otherwise as said before - go ahead, I think this would be great to
have for GCC 8.  I'd say
ask the maintainers of the above list of targets to do some testing.

"Fixing" POINTER_PLUS_EXPR would be very nice as well.  Again, matching
precision - I'm not sure if we need to force a signed operand, having either
might be nice given all sizes are usually unsigned.

Thanks and sorry for the delay,
Richard.

> --
> Marc Glisse
Marc Glisse Oct. 28, 2017, 12:53 p.m. UTC | #4
I am sending the new version of the patch in a separate email, to make it 
more visible, and only replying to a few points here.

On Thu, 19 Oct 2017, Richard Biener wrote:

> On Mon, Oct 9, 2017 at 12:55 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> On Mon, 3 Jul 2017, Richard Biener wrote:
>>
>>> On Sat, 1 Jul 2017, Marc Glisse wrote:
>>>
>>>> I wrote a quick prototype to see what the fallout would look like.
>>>> Surprisingly, it actually passes bootstrap+testsuite on ppc64el with all
>>>> languages with no regression. Sure, it is probably not a complete
>>>> migration, there are likely a few places still converting to ptrdiff_t
>>>> to perform a regular subtraction, but this seems to indicate that the
>>>> work isn't as bad as using a signed type in pointer_plus_expr for
>>>> instance.
>>>
>>>
>>> The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
>>> from POINTER_MINUS_EXPR in some cases I guess).
>>>
>>> The tree code needs documenting in tree.def and generic.texi.
>>>
>>> Otherwise ok(*).
>>>
>>> Thanks,
>>> Richard.
>>>
>>> (*) ok, just kidding -- or maybe not
>>
>>
>> I updated the prototype a bit. Some things I noticed:
>>
>> - the C front-end has support for address spaces that seems to imply that
>> pointer subtraction (and division by the size) may be done in a type larger
>> than ptrdiff_t. Currently, it generates
>> (ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is some type
>> potentially larger than ptrdiff_t.
>
> It uses a ptrdiff_t corresponding to the respective address space, yes.
> That we use sizetype elsewhere unconditionally is a bug :/
>
>> I am thus generating a POINTER_DIFF_EXPR
>> with that type, while I was originally hoping its type would always be
>> ptrdiff_t. It complicates code and I am not sure I didn't break address
>> spaces anyway... (expansion likely needs to do the inttype dance)
>
> I think that's fine.  Ideally targets would provide a type to use for each
> respective address space given we have targets that have sizetype smaller
> than ptr_mode.
>
>> Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t (what
>> POINTER_PLUS_EXPR takes as second argument) always the same type
>> signed/unsigned?
>
> POINTER_PLUS_EXPR takes 'sizetype', not size_t.

Ah, yes, that's the one I meant...

> So the answer is clearly no.  And yes, that's ugly and broken and should be fixed.
>
>> Counter-examples: m32c (when !TARGET_A16), visium, darwin,
>> powerpcspe, s390, vms... and it isn't even always the same that is bigger
>> than the other. That's quite messy.
>
> Eh.  Yeah, targets are free to choose 'sizetype' and they do so for
> efficiency.  IMHO we should get rid of this "feature".
>
>> - I had to use @@ in match.pd, not for constants, but because in GENERIC we
>> sometimes ended up with pointers where operand_equal_p said yes but
>> types_match disagreed.
>>
>> - It currently regresses 2 go tests: net/http runtime/debug
>
> Those are flaky for me and fail sometimes and sometimes not.

Yes, I noticed that (there are 1 or 2 others in the go testsuite).

> +@item POINTER_DIFF_EXPR
> +This node represents pointer subtraction.  The two operands always
> +have pointer/reference type.  The second operand is always an unsigned
> +integer type compatible with sizetype.  It returns a signed integer.
>
> the 2nd sentence looks bogusly copied.

Oups, removed.

>
> +      /* FIXME.  */
> +      if (code == POINTER_DIFF_EXPR)
> +       return int_const_binop (MINUS_EXPR,
> +                               fold_convert (ptrdiff_type_node, arg1),
> +                               fold_convert (ptrdiff_type_node, arg2));
>
>  wide_int_to_tree (type, wi::to_widest (arg1) - wi::to_widest (arg2));

Before your reply, I wrote something similar using offset_int instead of 
widest_int (and handling overflow, mostly for documentation purposes). I 
wasn't sure which one to pick, I can change to widest_int if you think it 
is better...

> +    case POINTER_DIFF_EXPR:
> +      {
> +       if (!POINTER_TYPE_P (rhs1_type)
> +           || !POINTER_TYPE_P (rhs2_type)
> +           // || !useless_type_conversion_p (rhs2_type, rhs1_type)
>
> types_compatible_p (rhs1_type, rhs2_type)?

Makes sense.

> +           // || !useless_type_conversion_p (ptrdiff_type_node, lhs_type))
> +           || TREE_CODE (lhs_type) != INTEGER_TYPE
> +           || TYPE_UNSIGNED (lhs_type))
> +         {
> +           error ("type mismatch in pointer diff expression");
> +           debug_generic_stmt (lhs_type);
> +           debug_generic_stmt (rhs1_type);
> +           debug_generic_stmt (rhs2_type);
> +           return true;
>
> there's also verify_expr which could want adjustment for newly created
> tree kinds.

ok.

> So if the precision of the result is larger than that of the pointer 
> operands we sign-extend the result, right?  So the subtraction is 
> performed in precision of the pointer operands and then 
> sign-extended/truncated to the result type? Which means it is _not_ a 
> widening subtraction to get around the half-address-space issue.  The 
> tree.def documentation should reflect this semantic detail.  Not sure if 
> the RTL expansion follows it.

I actually have no idea what expansion does if the size is different (that 
was one of my comments). Crashing is not unlikely.

I have changed things a bit. Now POINTER_DIFF_EXPR always has the same 
precision as input and output (so expansion should be fine). And there is 
code in the C/C++ front-ends that uses casts and MINUS_EXPR if we want a 
result wider than pointers, although it might very well be dead code...

My goal was not to help with objects larger than half the address space, 
but to fix things for small objects unlucky enough to straddle the middle 
of the address space. By properly modeling what overflow means for this 
operation, I expect we can add more simplifications in match.pd.

> I think that we'd ideally have a single legal INTEGER_TYPE precision
> per pointer type precision and that those precisions should match...
> we don't have to follow the mistakes of POINTER_PLUS_EXPR.
>
> So ... above verify TYPE_PRECISION (rhs1_type) == TYPE_PRECISION (lhs_type)?
> Some targets have 24bit ptr_mode but no 24bit integer type which means the
> FE likely chooses 32bit int for the difference computation.  But the middle-end
> should be free to create a 24bit precision type with SImode.
>
> Otherwise as said before - go ahead, I think this would be great to
> have for GCC 8.  I'd say
> ask the maintainers of the above list of targets to do some testing.
>
> "Fixing" POINTER_PLUS_EXPR would be very nice as well.  Again, matching
> precision - I'm not sure if we need to force a signed operand, having either
> might be nice given all sizes are usually unsigned.
>
> Thanks and sorry for the delay,
> Richard.
Richard Biener Oct. 28, 2017, 4:45 p.m. UTC | #5
On October 28, 2017 2:53:56 PM GMT+02:00, Marc Glisse <marc.glisse@inria.fr> wrote:
>
>I am sending the new version of the patch in a separate email, to make
>it 
>more visible, and only replying to a few points here.
>
>On Thu, 19 Oct 2017, Richard Biener wrote:
>
>> On Mon, Oct 9, 2017 at 12:55 PM, Marc Glisse <marc.glisse@inria.fr>
>wrote:
>>> On Mon, 3 Jul 2017, Richard Biener wrote:
>>>
>>>> On Sat, 1 Jul 2017, Marc Glisse wrote:
>>>>
>>>>> I wrote a quick prototype to see what the fallout would look like.
>>>>> Surprisingly, it actually passes bootstrap+testsuite on ppc64el
>with all
>>>>> languages with no regression. Sure, it is probably not a complete
>>>>> migration, there are likely a few places still converting to
>ptrdiff_t
>>>>> to perform a regular subtraction, but this seems to indicate that
>the
>>>>> work isn't as bad as using a signed type in pointer_plus_expr for
>>>>> instance.
>>>>
>>>>
>>>> The fold_binary_loc hunk looks dangerous (it'll generate MINUS_EXPR
>>>> from POINTER_MINUS_EXPR in some cases I guess).
>>>>
>>>> The tree code needs documenting in tree.def and generic.texi.
>>>>
>>>> Otherwise ok(*).
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>> (*) ok, just kidding -- or maybe not
>>>
>>>
>>> I updated the prototype a bit. Some things I noticed:
>>>
>>> - the C front-end has support for address spaces that seems to imply
>that
>>> pointer subtraction (and division by the size) may be done in a type
>larger
>>> than ptrdiff_t. Currently, it generates
>>> (ptrdiff_t)(((inttype)q-(inttype)p)/size) for q-p where inttype is
>some type
>>> potentially larger than ptrdiff_t.
>>
>> It uses a ptrdiff_t corresponding to the respective address space,
>yes.
>> That we use sizetype elsewhere unconditionally is a bug :/
>>
>>> I am thus generating a POINTER_DIFF_EXPR
>>> with that type, while I was originally hoping its type would always
>be
>>> ptrdiff_t. It complicates code and I am not sure I didn't break
>address
>>> spaces anyway... (expansion likely needs to do the inttype dance)
>>
>> I think that's fine.  Ideally targets would provide a type to use for
>each
>> respective address space given we have targets that have sizetype
>smaller
>> than ptr_mode.
>>
>>> Are ptrdiff_t (what POINTER_DIFF_EXPR should return) and size_t
>(what
>>> POINTER_PLUS_EXPR takes as second argument) always the same type
>>> signed/unsigned?
>>
>> POINTER_PLUS_EXPR takes 'sizetype', not size_t.
>
>Ah, yes, that's the one I meant...
>
>> So the answer is clearly no.  And yes, that's ugly and broken and
>should be fixed.
>>
>>> Counter-examples: m32c (when !TARGET_A16), visium, darwin,
>>> powerpcspe, s390, vms... and it isn't even always the same that is
>bigger
>>> than the other. That's quite messy.
>>
>> Eh.  Yeah, targets are free to choose 'sizetype' and they do so for
>> efficiency.  IMHO we should get rid of this "feature".
>>
>>> - I had to use @@ in match.pd, not for constants, but because in
>GENERIC we
>>> sometimes ended up with pointers where operand_equal_p said yes but
>>> types_match disagreed.
>>>
>>> - It currently regresses 2 go tests: net/http runtime/debug
>>
>> Those are flaky for me and fail sometimes and sometimes not.
>
>Yes, I noticed that (there are 1 or 2 others in the go testsuite).
>
>> +@item POINTER_DIFF_EXPR
>> +This node represents pointer subtraction.  The two operands always
>> +have pointer/reference type.  The second operand is always an
>unsigned
>> +integer type compatible with sizetype.  It returns a signed integer.
>>
>> the 2nd sentence looks bogusly copied.
>
>Oups, removed.
>
>>
>> +      /* FIXME.  */
>> +      if (code == POINTER_DIFF_EXPR)
>> +       return int_const_binop (MINUS_EXPR,
>> +                               fold_convert (ptrdiff_type_node,
>arg1),
>> +                               fold_convert (ptrdiff_type_node,
>arg2));
>>
>>  wide_int_to_tree (type, wi::to_widest (arg1) - wi::to_widest
>(arg2));
>
>Before your reply, I wrote something similar using offset_int instead
>of 
>widest_int (and handling overflow, mostly for documentation purposes).
>I 
>wasn't sure which one to pick, I can change to widest_int if you think
>it 
>is better...

Offset_int is better. 

>> +    case POINTER_DIFF_EXPR:
>> +      {
>> +       if (!POINTER_TYPE_P (rhs1_type)
>> +           || !POINTER_TYPE_P (rhs2_type)
>> +           // || !useless_type_conversion_p (rhs2_type, rhs1_type)
>>
>> types_compatible_p (rhs1_type, rhs2_type)?
>
>Makes sense.
>
>> +           // || !useless_type_conversion_p (ptrdiff_type_node,
>lhs_type))
>> +           || TREE_CODE (lhs_type) != INTEGER_TYPE
>> +           || TYPE_UNSIGNED (lhs_type))
>> +         {
>> +           error ("type mismatch in pointer diff expression");
>> +           debug_generic_stmt (lhs_type);
>> +           debug_generic_stmt (rhs1_type);
>> +           debug_generic_stmt (rhs2_type);
>> +           return true;
>>
>> there's also verify_expr which could want adjustment for newly
>created
>> tree kinds.
>
>ok.
>
>> So if the precision of the result is larger than that of the pointer 
>> operands we sign-extend the result, right?  So the subtraction is 
>> performed in precision of the pointer operands and then 
>> sign-extended/truncated to the result type? Which means it is _not_ a
>
>> widening subtraction to get around the half-address-space issue.  The
>
>> tree.def documentation should reflect this semantic detail.  Not sure
>if 
>> the RTL expansion follows it.
>
>I actually have no idea what expansion does if the size is different
>(that 
>was one of my comments). Crashing is not unlikely.
>
>I have changed things a bit. Now POINTER_DIFF_EXPR always has the same 
>precision as input and output (so expansion should be fine). And there
>is 
>code in the C/C++ front-ends that uses casts and MINUS_EXPR if we want
>a 
>result wider than pointers, although it might very well be dead code...
>
>My goal was not to help with objects larger than half the address
>space, 
>but to fix things for small objects unlucky enough to straddle the
>middle 
>of the address space. By properly modeling what overflow means for this
>
>operation, I expect we can add more simplifications in match.pd.

Yeah. 

Thanks, 
Richard. 

>> I think that we'd ideally have a single legal INTEGER_TYPE precision
>> per pointer type precision and that those precisions should match...
>> we don't have to follow the mistakes of POINTER_PLUS_EXPR.
>>
>> So ... above verify TYPE_PRECISION (rhs1_type) == TYPE_PRECISION
>(lhs_type)?
>> Some targets have 24bit ptr_mode but no 24bit integer type which
>means the
>> FE likely chooses 32bit int for the difference computation.  But the
>middle-end
>> should be free to create a 24bit precision type with SImode.
>>
>> Otherwise as said before - go ahead, I think this would be great to
>> have for GCC 8.  I'd say
>> ask the maintainers of the above list of targets to do some testing.
>>
>> "Fixing" POINTER_PLUS_EXPR would be very nice as well.  Again,
>matching
>> precision - I'm not sure if we need to force a signed operand, having
>either
>> might be nice given all sizes are usually unsigned.
>>
>> Thanks and sorry for the delay,
>> Richard.
diff mbox

Patch

Index: gcc/c/c-fold.c
===================================================================
--- gcc/c/c-fold.c	(revision 249856)
+++ gcc/c/c-fold.c	(working copy)
@@ -238,20 +238,21 @@  c_fully_fold_internal (tree expr, bool i
     case COMPOUND_EXPR:
     case MODIFY_EXPR:
     case PREDECREMENT_EXPR:
     case PREINCREMENT_EXPR:
     case POSTDECREMENT_EXPR:
     case POSTINCREMENT_EXPR:
     case PLUS_EXPR:
     case MINUS_EXPR:
     case MULT_EXPR:
     case POINTER_PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case TRUNC_DIV_EXPR:
     case CEIL_DIV_EXPR:
     case FLOOR_DIV_EXPR:
     case TRUNC_MOD_EXPR:
     case RDIV_EXPR:
     case EXACT_DIV_EXPR:
     case LSHIFT_EXPR:
     case RSHIFT_EXPR:
     case BIT_IOR_EXPR:
     case BIT_XOR_EXPR:
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 249856)
+++ gcc/c/c-typeck.c	(working copy)
@@ -3820,23 +3820,21 @@  pointer_diff (location_t loc, tree op0,
 	     "pointer of type %<void *%> used in subtraction");
   if (TREE_CODE (target_type) == FUNCTION_TYPE)
     pedwarn (loc, OPT_Wpointer_arith,
 	     "pointer to a function used in subtraction");
 
   /* First do the subtraction as integers;
      then drop through to build the divide operator.
      Do not do default conversions on the minus operator
      in case restype is a short type.  */
 
-  op0 = build_binary_op (loc,
-			 MINUS_EXPR, convert (inttype, op0),
-			 convert (inttype, op1), 0);
+  op0 = build2_loc (loc, POINTER_DIFF_EXPR, ptrdiff_type_node, op0, op1);
   /* This generates an error if op1 is pointer to incomplete type.  */
   if (!COMPLETE_OR_VOID_TYPE_P (TREE_TYPE (TREE_TYPE (orig_op1))))
     error_at (loc, "arithmetic on pointer to an incomplete type");
 
   op1 = c_size_in_bytes (target_type);
 
   if (pointer_to_zero_sized_aggr_p (TREE_TYPE (orig_op1)))
     error_at (loc, "arithmetic on pointer to an empty aggregate");
 
   /* Divide by the size, in easiest possible way.  */
@@ -9967,20 +9965,21 @@  c_finish_return (location_t loc, tree re
 	{
 	  switch (TREE_CODE (inner))
 	    {
 	    CASE_CONVERT:
 	    case NON_LVALUE_EXPR:
 	    case PLUS_EXPR:
 	    case POINTER_PLUS_EXPR:
 	      inner = TREE_OPERAND (inner, 0);
 	      continue;
 
+	    case POINTER_DIFF_EXPR:
 	    case MINUS_EXPR:
 	      /* If the second operand of the MINUS_EXPR has a pointer
 		 type (or is converted from it), this may be valid, so
 		 don't give a warning.  */
 	      {
 		tree op1 = TREE_OPERAND (inner, 1);
 
 		while (!POINTER_TYPE_P (TREE_TYPE (op1))
 		       && (CONVERT_EXPR_P (op1)
 			   || TREE_CODE (op1) == NON_LVALUE_EXPR))
Index: gcc/c-family/c-pretty-print.c
===================================================================
--- gcc/c-family/c-pretty-print.c	(revision 249856)
+++ gcc/c-family/c-pretty-print.c	(working copy)
@@ -1863,20 +1863,21 @@  c_pretty_printer::multiplicative_express
       additive-expression - multiplicative-expression   */
 
 static void
 pp_c_additive_expression (c_pretty_printer *pp, tree e)
 {
   enum tree_code code = TREE_CODE (e);
   switch (code)
     {
     case POINTER_PLUS_EXPR:
     case PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       pp_c_additive_expression (pp, TREE_OPERAND (e, 0));
       pp_c_whitespace (pp);
       if (code == PLUS_EXPR || code == POINTER_PLUS_EXPR)
 	pp_plus (pp);
       else
 	pp_minus (pp);
       pp_c_whitespace (pp);
       pp->multiplicative_expression (TREE_OPERAND (e, 1));
       break;
@@ -2279,20 +2280,21 @@  c_pretty_printer::expression (tree e)
     case NE_EXPR:
       pp_c_equality_expression (this, e);
       break;
 
     case COND_EXPR:
       conditional_expression (e);
       break;
 
     case POINTER_PLUS_EXPR:
     case PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       pp_c_additive_expression (this, e);
       break;
 
     case MODIFY_EXPR:
     case INIT_EXPR:
       assignment_expression (e);
       break;
 
     case COMPOUND_EXPR:
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 249856)
+++ gcc/cfgexpand.c	(working copy)
@@ -4595,20 +4595,21 @@  expand_debug_expr (tree exp)
 	       the operand, because the operand is always unsigned
 	       here even if the original C expression is signed.  */
 	    op1 = simplify_gen_unary (SIGN_EXTEND, GET_MODE (op0), op1,
 				      GET_MODE (op1));
 	}
       /* Fall through.  */
     case PLUS_EXPR:
       return simplify_gen_binary (PLUS, mode, op0, op1);
 
     case MINUS_EXPR:
+    case POINTER_DIFF_EXPR:
       return simplify_gen_binary (MINUS, mode, op0, op1);
 
     case MULT_EXPR:
       return simplify_gen_binary (MULT, mode, op0, op1);
 
     case RDIV_EXPR:
     case TRUNC_DIV_EXPR:
     case EXACT_DIV_EXPR:
       if (unsignedp)
 	return simplify_gen_binary (UDIV, mode, op0, op1);
Index: gcc/cp/constexpr.c
===================================================================
--- gcc/cp/constexpr.c	(revision 249856)
+++ gcc/cp/constexpr.c	(working copy)
@@ -4247,20 +4247,21 @@  cxx_eval_constant_expression (const cons
 	      return t;
 	    op1 = TREE_OPERAND (t, 1);
 	    r = cxx_eval_constant_expression (ctx, op1,
 					      lval, non_constant_p, overflow_p,
 					      jump_target);
 	  }
       }
       break;
 
     case POINTER_PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case PLUS_EXPR:
     case MINUS_EXPR:
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case CEIL_DIV_EXPR:
     case FLOOR_DIV_EXPR:
     case ROUND_DIV_EXPR:
     case TRUNC_MOD_EXPR:
     case CEIL_MOD_EXPR:
     case ROUND_MOD_EXPR:
@@ -5460,20 +5461,21 @@  potential_constant_expression_1 (tree t,
 	    && TYPE_POLYMORPHIC_P (TREE_TYPE (e)))
           {
             if (flags & tf_error)
               error_at (loc, "typeid-expression is not a constant expression "
 			"because %qE is of polymorphic type", e);
             return false;
           }
         return true;
       }
 
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       want_rval = true;
       goto binary;
 
     case LT_EXPR:
     case LE_EXPR:
     case GT_EXPR:
     case GE_EXPR:
     case EQ_EXPR:
     case NE_EXPR:
Index: gcc/cp/cp-gimplify.c
===================================================================
--- gcc/cp/cp-gimplify.c	(revision 249856)
+++ gcc/cp/cp-gimplify.c	(working copy)
@@ -2198,20 +2198,21 @@  cp_fold (tree x)
     case POSTINCREMENT_EXPR:
     case INIT_EXPR:
     case PREDECREMENT_EXPR:
     case PREINCREMENT_EXPR:
     case COMPOUND_EXPR:
     case MODIFY_EXPR:
       rval_ops = false;
       /* FALLTHRU */
     case POINTER_PLUS_EXPR:
     case PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
     case MULT_EXPR:
     case TRUNC_DIV_EXPR:
     case CEIL_DIV_EXPR:
     case FLOOR_DIV_EXPR:
     case ROUND_DIV_EXPR:
     case TRUNC_MOD_EXPR:
     case CEIL_MOD_EXPR:
     case ROUND_MOD_EXPR:
     case RDIV_EXPR:
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 249856)
+++ gcc/cp/typeck.c	(working copy)
@@ -4303,20 +4303,21 @@  cp_build_binary_op (location_t location,
               converted = 1;
               break;
             }
           default:
             break;
         }
     }
 
   switch (code)
     {
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       /* Subtraction of two similar pointers.
 	 We must subtract them as integers, then divide by object size.  */
       if (code0 == POINTER_TYPE && code1 == POINTER_TYPE
 	  && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (type0),
 							TREE_TYPE (type1)))
 	return pointer_diff (location, op0, op1,
 			     common_pointer_type (type0, type1), complain);
       /* In all other cases except pointer - int, the usual arithmetic
 	 rules apply.  */
@@ -5394,25 +5395,26 @@  pointer_diff (location_t loc, tree op0,
       if (complain & tf_error)
 	permerror (loc, "ISO C++ forbids using pointer to "
 		   "a method in subtraction");
       else
 	return error_mark_node;
     }
 
   /* First do the subtraction as integers;
      then drop through to build the divide operator.  */
 
-  op0 = cp_build_binary_op (loc,
-			    MINUS_EXPR,
-			    cp_convert (restype, op0, complain),
-			    cp_convert (restype, op1, complain),
-			    complain);
+  op0 = build2_loc (loc,
+			    POINTER_DIFF_EXPR,
+			    ssizetype,
+			    op0,
+			    op1
+			    );
 
   /* This generates an error if op1 is a pointer to an incomplete type.  */
   if (!COMPLETE_TYPE_P (TREE_TYPE (TREE_TYPE (op1))))
     {
       if (complain & tf_error)
 	error_at (loc, "invalid use of a pointer to an incomplete type in "
 		  "pointer arithmetic");
       else
 	return error_mark_node;
     }
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 249856)
+++ gcc/expr.c	(working copy)
@@ -8537,20 +8537,21 @@  expand_expr_real_2 (sepops ops, rtx targ
 	  if (op1 == const0_rtx)
 	    return op0;
 	  goto binop2;
 	}
 
       expand_operands (treeop0, treeop1,
 		       subtarget, &op0, &op1, modifier);
       return REDUCE_BIT_FIELD (simplify_gen_binary (PLUS, mode, op0, op1));
 
     case MINUS_EXPR:
+    case POINTER_DIFF_EXPR:
     do_minus:
       /* For initializers, we are allowed to return a MINUS of two
 	 symbolic constants.  Here we handle all cases when both operands
 	 are constant.  */
       /* Handle difference of two symbolic constants,
 	 for the sake of an initializer.  */
       if ((modifier == EXPAND_SUM || modifier == EXPAND_INITIALIZER)
 	  && really_constant_p (treeop0)
 	  && really_constant_p (treeop1))
 	{
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 249856)
+++ gcc/fold-const.c	(working copy)
@@ -1138,20 +1138,24 @@  const_binop (enum tree_code code, tree a
     return NULL_TREE;
 
   STRIP_NOPS (arg1);
   STRIP_NOPS (arg2);
 
   if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg2) == INTEGER_CST)
     {
       if (code == POINTER_PLUS_EXPR)
 	return int_const_binop (PLUS_EXPR,
 				arg1, fold_convert (TREE_TYPE (arg1), arg2));
+      if (code == POINTER_DIFF_EXPR)
+	return int_const_binop (MINUS_EXPR,
+				fold_convert (ptrdiff_type_node, arg1),
+				fold_convert (ptrdiff_type_node, arg2));
 
       return int_const_binop (code, arg1, arg2);
     }
 
   if (TREE_CODE (arg1) == REAL_CST && TREE_CODE (arg2) == REAL_CST)
     {
       machine_mode mode;
       REAL_VALUE_TYPE d1;
       REAL_VALUE_TYPE d2;
       REAL_VALUE_TYPE value;
@@ -9764,20 +9768,21 @@  fold_binary_loc (location_t loc,
 
 	      con0 = associate_trees (loc, con0, lit0, code, atype);
 	      return
 		fold_convert_loc (loc, type, associate_trees (loc, var0, con0,
 							      code, atype));
 	    }
 	}
 
       return NULL_TREE;
 
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       /* (-A) - B -> (-B) - A  where B is easily negated and we can swap.  */
       if (TREE_CODE (arg0) == NEGATE_EXPR
 	  && negate_expr_p (op1))
 	return fold_build2_loc (loc, MINUS_EXPR, type,
 				negate_expr (op1),
 				fold_convert_loc (loc, type,
 						  TREE_OPERAND (arg0, 0)));
 
       /* Fold __complex__ ( x, 0 ) - __complex__ ( 0, y ) to
Index: gcc/match.pd
===================================================================
--- gcc/match.pd	(revision 249856)
+++ gcc/match.pd	(working copy)
@@ -117,20 +117,23 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 
 /* Simplify x - x.
    This is unsafe for certain floats even in non-IEEE formats.
    In IEEE, it is unsafe because it does wrong for NaNs.
    Also note that operand_equal_p is always false if an operand
    is volatile.  */
 (simplify
  (minus @0 @0)
  (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (type))
   { build_zero_cst (type); }))
+(simplify
+ (pointer_diff @0 @0)
+ { build_zero_cst (type); })
 
 (simplify
  (mult @0 integer_zerop@1)
  @1)
 
 /* Maybe fold x * 0 to 0.  The expressions aren't the same
    when x is NaN, since x * 0 is also NaN.  Nor are they the
    same in modes with signed zeros, since multiplying a
    negative value by 0 gives -0, not +0.  */
 (simplify
@@ -1269,20 +1272,27 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
      tem5 = ptr1 + tem4;
    and produce
      tem5 = ptr2;  */
 (simplify
   (pointer_plus @0 (convert?@2 (minus@3 (convert @1) (convert @0))))
   /* Conditionally look through a sign-changing conversion.  */
   (if (TYPE_PRECISION (TREE_TYPE (@2)) == TYPE_PRECISION (TREE_TYPE (@3))
        && ((GIMPLE && useless_type_conversion_p (type, TREE_TYPE (@1)))
 	    || (GENERIC && type == TREE_TYPE (@1))))
    @1))
+(simplify
+  (pointer_plus @0 (convert?@2 (pointer_diff@3 @1 @0)))
+  /* Conditionally look through a sign-changing conversion.  */
+  (if (TYPE_PRECISION (TREE_TYPE (@2)) == TYPE_PRECISION (TREE_TYPE (@3))
+       && ((GIMPLE && useless_type_conversion_p (type, TREE_TYPE (@1)))
+	    || (GENERIC && type == TREE_TYPE (@1))))
+   @1))
 
 /* Pattern match
      tem = (sizetype) ptr;
      tem = tem & algn;
      tem = -tem;
      ... = ptr p+ tem;
    and produce the simpler and easier to analyze with respect to alignment
      ... = ptr & ~algn;  */
 (simplify
   (pointer_plus @0 (negate (bit_and (convert @0) INTEGER_CST@1)))
@@ -1295,20 +1305,34 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
   (with { HOST_WIDE_INT diff; }
    (if (ptr_difference_const (@0, @1, &diff))
     { build_int_cst_type (type, diff); }))))
 (simplify
  (minus (convert @0) (convert ADDR_EXPR@1))
  (if (tree_nop_conversion_p (type, TREE_TYPE (@0)))
   (with { HOST_WIDE_INT diff; }
    (if (ptr_difference_const (@0, @1, &diff))
     { build_int_cst_type (type, diff); }))))
+(simplify
+ (pointer_diff (convert?@2 ADDR_EXPR@0) (convert?@3 @1))
+ (if (tree_nop_conversion_p (TREE_TYPE(@2), TREE_TYPE (@0))
+      && tree_nop_conversion_p (TREE_TYPE(@3), TREE_TYPE (@1)))
+  (with { HOST_WIDE_INT diff; }
+   (if (ptr_difference_const (@0, @1, &diff))
+    { build_int_cst_type (type, diff); }))))
+(simplify
+ (pointer_diff (convert?@2 @0) (convert?@3 ADDR_EXPR@1))
+ (if (tree_nop_conversion_p (TREE_TYPE(@2), TREE_TYPE (@0))
+      && tree_nop_conversion_p (TREE_TYPE(@3), TREE_TYPE (@1)))
+  (with { HOST_WIDE_INT diff; }
+   (if (ptr_difference_const (@0, @1, &diff))
+    { build_int_cst_type (type, diff); }))))
 
 /* If arg0 is derived from the address of an object or function, we may
    be able to fold this expression using the object or function's
    alignment.  */
 (simplify
  (bit_and (convert? @0) INTEGER_CST@1)
  (if (POINTER_TYPE_P (TREE_TYPE (@0))
       && tree_nop_conversion_p (type, TREE_TYPE (@0)))
   (with
    {
@@ -1491,20 +1515,29 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
 	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
 	 /* For pointer types, if the conversion of A to the
 	    final type requires a sign- or zero-extension,
 	    then we have to punt - it is not defined which
 	    one is correct.  */
 	 || (POINTER_TYPE_P (TREE_TYPE (@0))
 	     && TREE_CODE (@1) == INTEGER_CST
 	     && tree_int_cst_sign_bit (@1) == 0))
      (convert @1))))
+   (simplify
+    (pointer_diff (pointer_plus @0 @1) @0)
+    (if (element_precision (type) <= element_precision (@1)
+	 || tree_expr_nonnegative_p (@1))
+	 /* For pointer types, if the conversion of A to the
+	    final type requires a sign- or zero-extension,
+	    then we have to punt - it is not defined which
+	    one is correct.  */
+     (convert @1)))
 
   /* (T)P - (T)(P + A) -> -(T) A */
   (for add (plus pointer_plus)
    (simplify
     (minus (convert @0)
      (convert (add @@0 @1)))
     (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
 	 /* For integer types, if A has a smaller type
 	    than T the result depends on the possible
 	    overflow in P + A.
@@ -1515,20 +1548,29 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 || (INTEGRAL_TYPE_P (TREE_TYPE (@0))
 	     && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (@0)))
 	 /* For pointer types, if the conversion of A to the
 	    final type requires a sign- or zero-extension,
 	    then we have to punt - it is not defined which
 	    one is correct.  */
 	 || (POINTER_TYPE_P (TREE_TYPE (@0))
 	     && TREE_CODE (@1) == INTEGER_CST
 	     && tree_int_cst_sign_bit (@1) == 0))
      (negate (convert @1)))))
+   (simplify
+    (pointer_diff @0 (pointer_plus @0 @1))
+    (if (element_precision (type) <= element_precision (@1)
+	 || tree_expr_nonnegative_p (@1))
+	 /* For pointer types, if the conversion of A to the
+	    final type requires a sign- or zero-extension,
+	    then we have to punt - it is not defined which
+	    one is correct.  */
+     (negate (convert @1))))
 
   /* (T)(P + A) - (T)(P + B) -> (T)A - (T)B */
   (for add (plus pointer_plus)
    (simplify
     (minus (convert (add @@0 @1))
      (convert (add @0 @2)))
     (if (element_precision (type) <= element_precision (TREE_TYPE (@1))
 	 /* For integer types, if A has a smaller type
 	    than T the result depends on the possible
 	    overflow in P + A.
@@ -1541,20 +1583,29 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 	 /* For pointer types, if the conversion of A to the
 	    final type requires a sign- or zero-extension,
 	    then we have to punt - it is not defined which
 	    one is correct.  */
 	 || (POINTER_TYPE_P (TREE_TYPE (@0))
 	     && TREE_CODE (@1) == INTEGER_CST
 	     && tree_int_cst_sign_bit (@1) == 0
 	     && TREE_CODE (@2) == INTEGER_CST
 	     && tree_int_cst_sign_bit (@2) == 0))
      (minus (convert @1) (convert @2)))))))
+   (simplify
+    (pointer_diff (pointer_plus @0 @1) (pointer_plus @0 @2))
+    (if (element_precision (type) <= element_precision (@1)
+	 || (tree_expr_nonnegative_p (@1) && tree_expr_nonnegative_p (@2)))
+	 /* For pointer types, if the conversion of A to the
+	    final type requires a sign- or zero-extension,
+	    then we have to punt - it is not defined which
+	    one is correct.  */
+     (minus (convert @1) (convert @2))))
 
 
 /* Simplifications of MIN_EXPR, MAX_EXPR, fmin() and fmax().  */
 
 (for minmax (min max FMIN FMAX)
  (simplify
   (minmax @0 @0)
   @0))
 /* min(max(x,y),y) -> y.  */
 (simplify
@@ -2524,20 +2575,25 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 /* Transform comparisons of the form X - Y CMP 0 to X CMP Y.
    ??? The transformation is valid for the other operators if overflow
    is undefined for the type, but performing it here badly interacts
    with the transformation in fold_cond_expr_with_comparison which
    attempts to synthetize ABS_EXPR.  */
 (for cmp (eq ne)
  (simplify
   (cmp (minus@2 @0 @1) integer_zerop)
   (if (single_use (@2))
    (cmp @0 @1))))
+(for cmp (eq ne)
+ (simplify
+  (cmp (pointer_diff@2 @0 @1) integer_zerop)
+  (if (single_use (@2))
+   (cmp @0 @1))))
 
 /* Transform comparisons of the form X * C1 CMP 0 to X CMP 0 in the
    signed arithmetic case.  That form is created by the compiler
    often enough for folding it to be of value.  One example is in
    computing loop trip counts after Operator Strength Reduction.  */
 (for cmp (simple_comparison)
      scmp (swapped_simple_comparison)
  (simplify
   (cmp (mult@3 @0 INTEGER_CST@1) integer_zerop@2)
   /* Handle unfolded multiplication by zero.  */
Index: gcc/optabs-tree.c
===================================================================
--- gcc/optabs-tree.c	(revision 249856)
+++ gcc/optabs-tree.c	(working copy)
@@ -216,20 +216,21 @@  optab_for_tree_code (enum tree_code code
 
   trapv = INTEGRAL_TYPE_P (type) && TYPE_OVERFLOW_TRAPS (type);
   switch (code)
     {
     case POINTER_PLUS_EXPR:
     case PLUS_EXPR:
       if (TYPE_SATURATING (type))
 	return TYPE_UNSIGNED (type) ? usadd_optab : ssadd_optab;
       return trapv ? addv_optab : add_optab;
 
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       if (TYPE_SATURATING (type))
 	return TYPE_UNSIGNED (type) ? ussub_optab : sssub_optab;
       return trapv ? subv_optab : sub_optab;
 
     case MULT_EXPR:
       if (TYPE_SATURATING (type))
 	return TYPE_UNSIGNED (type) ? usmul_optab : ssmul_optab;
       return trapv ? smulv_optab : smul_optab;
 
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 249856)
+++ gcc/tree-cfg.c	(working copy)
@@ -3966,20 +3966,37 @@  verify_gimple_assign_binary (gassign *st
 	    error ("type mismatch in pointer plus expression");
 	    debug_generic_stmt (lhs_type);
 	    debug_generic_stmt (rhs1_type);
 	    debug_generic_stmt (rhs2_type);
 	    return true;
 	  }
 
 	return false;
       }
 
+    case POINTER_DIFF_EXPR:
+      {
+	if (!POINTER_TYPE_P (rhs1_type)
+	    || !POINTER_TYPE_P (rhs2_type)
+	    // || !useless_type_conversion_p (rhs2_type, rhs1_type)
+	    || !useless_type_conversion_p (ptrdiff_type_node, lhs_type))
+	  {
+	    error ("type mismatch in pointer diff expression");
+	    debug_generic_stmt (lhs_type);
+	    debug_generic_stmt (rhs1_type);
+	    debug_generic_stmt (rhs2_type);
+	    return true;
+	  }
+
+	return false;
+      }
+
     case TRUTH_ANDIF_EXPR:
     case TRUTH_ORIF_EXPR:
     case TRUTH_AND_EXPR:
     case TRUTH_OR_EXPR:
     case TRUTH_XOR_EXPR:
 
       gcc_unreachable ();
 
     case LT_EXPR:
     case LE_EXPR:
Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c	(revision 249856)
+++ gcc/tree-inline.c	(working copy)
@@ -3906,20 +3906,21 @@  estimate_operator_cost (enum tree_code c
       return 0;
 
     /* Assign cost of 1 to usual operations.
        ??? We may consider mapping RTL costs to this.  */
     case COND_EXPR:
     case VEC_COND_EXPR:
     case VEC_PERM_EXPR:
 
     case PLUS_EXPR:
     case POINTER_PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
     case MULT_EXPR:
     case MULT_HIGHPART_EXPR:
     case FMA_EXPR:
 
     case ADDR_SPACE_CONVERT_EXPR:
     case FIXED_CONVERT_EXPR:
     case FIX_TRUNC_EXPR:
 
     case NEGATE_EXPR:
Index: gcc/tree-pretty-print.c
===================================================================
--- gcc/tree-pretty-print.c	(revision 249856)
+++ gcc/tree-pretty-print.c	(working copy)
@@ -2311,20 +2311,21 @@  dump_generic_node (pretty_printer *pp, t
       pp_greater (pp);
       break;
 
       /* Binary arithmetic and logic expressions.  */
     case WIDEN_SUM_EXPR:
     case WIDEN_MULT_EXPR:
     case MULT_EXPR:
     case MULT_HIGHPART_EXPR:
     case PLUS_EXPR:
     case POINTER_PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
     case TRUNC_DIV_EXPR:
     case CEIL_DIV_EXPR:
     case FLOOR_DIV_EXPR:
     case ROUND_DIV_EXPR:
     case TRUNC_MOD_EXPR:
     case CEIL_MOD_EXPR:
     case FLOOR_MOD_EXPR:
     case ROUND_MOD_EXPR:
     case RDIV_EXPR:
@@ -3546,20 +3547,21 @@  op_code_prio (enum tree_code code)
     case LROTATE_EXPR:
     case RROTATE_EXPR:
     case VEC_WIDEN_LSHIFT_HI_EXPR:
     case VEC_WIDEN_LSHIFT_LO_EXPR:
     case WIDEN_LSHIFT_EXPR:
       return 11;
 
     case WIDEN_SUM_EXPR:
     case PLUS_EXPR:
     case POINTER_PLUS_EXPR:
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       return 12;
 
     case VEC_WIDEN_MULT_HI_EXPR:
     case VEC_WIDEN_MULT_LO_EXPR:
     case WIDEN_MULT_EXPR:
     case DOT_PROD_EXPR:
     case WIDEN_MULT_PLUS_EXPR:
     case WIDEN_MULT_MINUS_EXPR:
     case MULT_EXPR:
@@ -3732,20 +3734,21 @@  op_symbol_code (enum tree_code code)
       return "w+";
 
     case WIDEN_MULT_EXPR:
       return "w*";
 
     case MULT_HIGHPART_EXPR:
       return "h*";
 
     case NEGATE_EXPR:
     case MINUS_EXPR:
+    case POINTER_DIFF_EXPR:
       return "-";
 
     case BIT_NOT_EXPR:
       return "~";
 
     case TRUTH_NOT_EXPR:
       return "!";
 
     case MULT_EXPR:
     case INDIRECT_REF:
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	(revision 249856)
+++ gcc/tree-vrp.c	(working copy)
@@ -3093,29 +3093,29 @@  extract_range_from_binary_expr (value_ra
 	set_value_range (&n_vr0, VR_RANGE, op0, op0, NULL);
 
       extract_range_from_binary_expr_1 (vr, code, expr_type, &n_vr0, &vr1);
     }
 
   /* If we didn't derive a range for MINUS_EXPR, and
      op1's range is ~[op0,op0] or vice-versa, then we
      can derive a non-null range.  This happens often for
      pointer subtraction.  */
   if (vr->type == VR_VARYING
-      && code == MINUS_EXPR
+      && (code == MINUS_EXPR || code == POINTER_DIFF_EXPR)
       && TREE_CODE (op0) == SSA_NAME
       && ((vr0.type == VR_ANTI_RANGE
 	   && vr0.min == op1
 	   && vr0.min == vr0.max)
 	  || (vr1.type == VR_ANTI_RANGE
 	      && vr1.min == op0
 	      && vr1.min == vr1.max)))
-      set_value_range_to_nonnull (vr, TREE_TYPE (op0));
+      set_value_range_to_nonnull (vr, expr_type);
 }
 
 /* Extract range information from a unary operation CODE based on
    the range of its operand *VR0 with type OP0_TYPE with resulting type TYPE.
    The resulting range is stored in *VR.  */
 
 void
 extract_range_from_unary_expr (value_range *vr,
 			       enum tree_code code, tree type,
 			       value_range *vr0_, tree op0_type)
Index: gcc/tree.def
===================================================================
--- gcc/tree.def	(revision 249856)
+++ gcc/tree.def	(working copy)
@@ -671,20 +671,22 @@  DEFTREECODE (PLACEHOLDER_EXPR, "placehol
 
 /* Simple arithmetic.  */
 DEFTREECODE (PLUS_EXPR, "plus_expr", tcc_binary, 2)
 DEFTREECODE (MINUS_EXPR, "minus_expr", tcc_binary, 2)
 DEFTREECODE (MULT_EXPR, "mult_expr", tcc_binary, 2)
 
 /* Pointer addition.  The first operand is always a pointer and the
    second operand is an integer of type sizetype.  */
 DEFTREECODE (POINTER_PLUS_EXPR, "pointer_plus_expr", tcc_binary, 2)
 
+DEFTREECODE (POINTER_DIFF_EXPR, "pointer_diff_expr", tcc_binary, 2)
+
 /* Highpart multiplication.  For an integral type with precision B,
    returns bits [2B-1, B] of the full 2*B product.  */
 DEFTREECODE (MULT_HIGHPART_EXPR, "mult_highpart_expr", tcc_binary, 2)
 
 /* Division for integer result that rounds the quotient toward zero.  */
 DEFTREECODE (TRUNC_DIV_EXPR, "trunc_div_expr", tcc_binary, 2)
 
 /* Division for integer result that rounds it toward plus infinity.  */
 DEFTREECODE (CEIL_DIV_EXPR, "ceil_div_expr", tcc_binary, 2)
 
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(revision 249856)
+++ gcc/varasm.c	(working copy)
@@ -4542,20 +4542,21 @@  initializer_constant_valid_p_1 (tree val
       else
       /* Support narrowing pointer differences.  */
 	ret = narrowing_initializer_constant_valid_p (value, endtype, NULL);
       if (cache)
 	{
 	  cache[0] = value;
 	  cache[1] = ret;
 	}
       return ret;
 
+    case POINTER_DIFF_EXPR:
     case MINUS_EXPR:
       if (TREE_CODE (endtype) == REAL_TYPE)
 	return NULL_TREE;
       if (cache && cache[0] == value)
 	return cache[1];
       if (! INTEGRAL_TYPE_P (endtype)
 	  || TYPE_PRECISION (endtype) >= TYPE_PRECISION (TREE_TYPE (value)))
 	{
 	  tree ncache[4] = { NULL_TREE, NULL_TREE, NULL_TREE, NULL_TREE };
 	  tree valid0