public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Marc Glisse <marc.glisse@inria.fr>
To: Richard Sandiford <richard.sandiford@arm.com>
Cc: Richard Biener <richard.guenther@gmail.com>,
	    GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Fold pointer range checks with equal spans
Date: Wed, 01 Aug 2018 12:18:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.21.1808011303330.25660@stedding.saclay.inria.fr> (raw)
In-Reply-To: <87pnz29wtd.fsf@arm.com>

On Wed, 1 Aug 2018, Richard Sandiford wrote:

> +/* For pointers @0 and @2 and nonnegative constant offset @1, look for
> +   expressions like:
> +
> +   A: (@0 + @1 < @2) | (@2 + @1 < @0)
> +   B: (@0 + @1 <= @2) | (@2 + @1 <= @0)

Once this is in, we may want to consider the opposite:

(@0 + @1 > @2) & (@2 + @1 > @0)

> +     /* Always fails for negative values.  */
> +     (if (wi::min_precision (rhs, UNSIGNED) <= TYPE_PRECISION (sizetype))

I guess we could simplify to 'true' in the 'else' case but that's less
interesting.

>> Turning multiple comparisons of the form P + cst CMP Q + cst into a
>> range check on P - Q sounds good (we don't really have to restrict to
>> the case where the range is symmetric). Actually, just turning P + cst
>> CMP Q + cst into P - Q CMP cst should do it, we should already have code
>> to handle range checking on integers (modulo the issue of CSE P-Q and
>> Q-P). But I don't know if a couple :s is sufficient to make this
>> transformation a good canonicalization.
>
> Yeah.  Like you say, in the cases being handled by the patch, folding the
> two comparisons separately and then folding the IOR would require either
>
> (a) folding:
>       P + cst < Q
>    to the rather unnatural-looking:
>       Q - P > -cst
>    when tree_swap_operands_p (P, Q) or

Is it unnatural? If it helps other optimizations, it doesn't look that
bad to me ;-)

One issue is if we start mixing forms because only one is single_use:
@0 + @1 < @2 | @1 < @0 - @2

> (b) making the range fold handle reversed pointer_diffs (which I guess
>    makes more sense).

It would be interesting to have a way to write:

(plus @0 (opposite@1 @0))

which would match if @0 is a-b and @1 is b-a or anything similar (I
don't want to repeat all the cases of negate, minus, pointer_diff, etc
in each transformation), but

(match (opposite (minus @0 @1)) (minus @1 @0))

is not a valid syntax. More likely we would write

(plus @0 @1) (if (opposite_p (@0, @1))

with opposite_p defined outside of match.pd :-(

Maybe there is a way to simulate binary predicates on @0 @1 using unary
predicates on (artificial @0 @1).

(looks like I forgot to add pointer_diff to negate_expr_p)

>> If we start from a comparison of pointer_plus, I think it would make
>> sense to use pointer_diff.
>>
>> I believe currently we try to use pointer operations (pointer_plus,
>> pointer_diff, lt) only for related pointers and we cast to some integer
>> type for wilder cases (implementation of std::less in C++ for instance).
>> On the other hand, in an alias check, the 2 pointers are possibly
>> unrelated, so maybe the code emitted for an alias check should be
>> changed.
>
> If we can prove that the pointers are to different objects, it would
> be valid to fold the check to "no alias", regardless of the constant
> (although in practice we should have weeded out those cases before
> generating the check).  In that sense, relying on the UBness of
> comparing pointers to different objects would be fine.  If there's a
> risk of the check being folded to "alias" when the pointers are known
> to point to different objects then that would be more of a problem
> (as a missed optimisation).

Assuming they are related, we are also assuming that pointer_diff will
not overflow. But for unrelated objects, in particular on 32bit targets,
pointer differences cannot all be represented by a 32 bit ptrdiff_t (the
differences live in a range twice that size), and doing comparisons on
it can have strange effects. In this particular case, I don't really see
a way things could break (you would somehow need one pointer close to 0
and the other close to 2^32 so they end up close modulo 2^32, but that
would require @2+@1 to overflow which means the loop will never run that
far anyway).

But we are still lying and taking a risk that some other optimization
will trust us. (I am ok with keeping the current alias code, just saying
that it involves a small, possibly negligible risk)

-- 
Marc Glisse

  reply	other threads:[~2018-08-01 12:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-20 10:27 Richard Sandiford
2018-07-20 12:11 ` Marc Glisse
2018-07-23 15:04   ` Richard Sandiford
2018-07-25  9:07     ` Richard Biener
2018-07-30 17:47       ` Richard Sandiford
2018-07-31 10:55         ` Richard Biener
2018-07-31 20:53           ` Marc Glisse
2018-08-01 10:59             ` Richard Sandiford
2018-08-01 12:18               ` Marc Glisse [this message]
2018-08-01 10:25           ` Richard Sandiford
2018-08-01 11:59             ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.21.1808011303330.25660@stedding.saclay.inria.fr \
    --to=marc.glisse@inria.fr \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.com \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).