public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Eric Botcazou <ebotcazou@adacore.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [RFC] optimize x - y cmp 0 with undefined overflow
Date: Tue, 27 May 2014 09:26:00 -0000	[thread overview]
Message-ID: <53896104.oWY7sHq2zD@polaris> (raw)
In-Reply-To: <CAFiYyc2OVqjwKMXn0Rz7-mN97xdV2=+11RvPO=619C6FOktQUA@mail.gmail.com>

> +      tree new_const
> +       = fold_build2_loc (loc, reverse_op, TREE_TYPE (arg1), const2,
> const1);
> 
>        /* If the constant operation overflowed this can be
>          simplified as a comparison against INT_MAX/INT_MIN.  */
> -      if (TREE_CODE (lhs) == INTEGER_CST
> -         && TREE_OVERFLOW (lhs))
> +      if (TREE_OVERFLOW (new_const))
> 
> well, either use int_const_binop above or retain the check (or use
> TREE_OVERFLOW_P).  Bonus points for using wide-ints here
> and not relying on TREE_OVERFLOW.

The check is useless (you get either NULL_TREE or INTEGER_CST here) but I'll 
use int_const_binop.

> +  /* Transform comparisons of the form X - Y CMP 0 to X CMP Y.  */
> +  if (TREE_CODE (arg0) == MINUS_EXPR
> +      && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (arg1))
> 
> any good reason for using TYPE_OVERFLOW_UNDEFINED on the
> type of arg1 instead on the type of the MINUS (yes, they should
> match, but it really looks odd ... the overflow of the minus has to be
> undefined).

For the sake of consistency with the X +- C1 CMP C2 case just above, but I can 
change both.

> Also for EQ_EXPR and NE_EXPR the transform is
> fine even when !TYPE_OVERFLOW_UNDEFINED (and we seem
> to perform it already somewhere).  Please look where and try to
> add the undefined overflow case to it.

Yes, but it's the same for the X +- C1 CMP C2 case, i.e. there are specific 
cases for X +- C1 EQ/NE C2 and X - Y EQ/NE 0 in fold_binary, so I'm not sure 
what you're asking.

> As for the VRP pieces I don't understand what is missing here
> (well, compare_range_with_value and/or compare_values might
> not handle this case?  then better fix that to improve symbolic
> range handling in general?).  Also I have a longstanding patch
> in my tree that does

Yes, there is an explicit non-handling of symbolic ranges for PLUS_EXPR and 
MINUS_EXPR in VRP (extract_range_from_binary_expr_1) and the patch works 
around it by propagating the code instead of the ranges, which is far easier 
and sufficient here.  If you think that the way to go is to handle symbolic 
ranges for PLUS_EXPR and MINUS_EXPR instead, fine with me, I can try.

-- 
Eric Botcazou

  reply	other threads:[~2014-05-27  9:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-26 10:24 Eric Botcazou
2014-05-26 12:55 ` Richard Biener
2014-05-27  9:26   ` Eric Botcazou [this message]
2014-05-27  9:42     ` Richard Biener
2014-05-27 10:00       ` Eric Botcazou
2014-05-27 10:12         ` Richard Biener
2014-05-30  8:49           ` Eric Botcazou
2014-06-02 10:36             ` Richard Biener
2014-06-02 10:37               ` Richard Biener
2014-06-03  8:13               ` Eric Botcazou
2014-06-03 11:00                 ` Richard Biener
2014-06-06 10:54                   ` Eric Botcazou
2014-06-06 15:45                     ` Richard Biener
2014-06-20  9:40                       ` Eric Botcazou
2014-06-24 10:34                         ` Richard Biener
2014-09-29 23:01                           ` Eric Botcazou
2014-05-27 16:14       ` Eric Botcazou
2014-05-27 16:19         ` 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=53896104.oWY7sHq2zD@polaris \
    --to=ebotcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.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).