From: Richard Biener <richard.guenther@gmail.com>
To: Robin Dapp <rdapp@linux.vnet.ibm.com>
Cc: "Bin.Cheng" <amker.cheng@gmail.com>,
GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 2/3] Simplify wrapped binops
Date: Tue, 27 Jun 2017 12:14:00 -0000 [thread overview]
Message-ID: <CAFiYyc0mkd=OQzJycfA42BWqkdb1XA8iBRS1KOSCUXbKkn=DTg@mail.gmail.com> (raw)
In-Reply-To: <3d769452-cd24-1f7b-6fb2-b556a51be55b@linux.vnet.ibm.com>
On Wed, Jun 21, 2017 at 1:44 PM, Robin Dapp <rdapp@linux.vnet.ibm.com> wrote:
>> use INTEGRAL_TYPE_P.
>
> Done.
>
>> but you do not actually _use_ vr_outer. Do you think that if
>> vr_outer is a VR_RANGE then the outer operation may not
>> possibly have wrapped? That's a false conclusion.
>
> These were remains of a previous version. vr_outer is indeed not needed
> anymore; removed.
>
>> wi::add overload with the overflow flag? ISTR you want to handle "negative"
>> unsigned constants somehow, but then I don't see how the above works.
>> I'd say if wmin/wmax interpreted as signed are positive and then using
>> a signed op to add w1 results in a still positive number you're fine
>> (you don't seem
>> to restrict the widening cast to either zero- or sign-extending).
>
> Changed to using wi:add overload now.
>
> In essence, three cases are being handled:
> - wrapped_range --> do not simplify
> - !wrapped_range && ovf ("negative" unsigned) --> simplify and combine
> with sign extension in the outer type
> - !wrapped_range && !ovf ("positive" unsigned) --> simplify and combine
> with zero extension in the outer type.
Let's split this and look at the simpler case:
+/* ((T)(A)) +- CST -> (T)(A +- CST) */
+#if GIMPLE
+ (for outer_op (plus minus)
+ (simplify
+ (outer_op (convert SSA_NAME@0) INTEGER_CST@2)
+ (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
+ && INTEGRAL_TYPE_P (type)
+ && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0)))
+ /* Perform binary operation inside the cast if the constant fits
+ and there is no overflow. */
+ (with
+ {
+ bool wrapped_range = true;
+ tree cst_inner = NULL_TREE;
+ enum value_range_type vr = VR_VARYING;
+ tree inner_type = TREE_TYPE (@0);
+
+ if (int_fits_type_p (@2, inner_type))
do
&& get_range_info (...) == VR_RANGE)
here. That avoids vr and its initialization and you get all of the "work" when
you know it will eventually succeed.
+ {
+ cst_inner = fold_convert (inner_type, @2);
ideally you'd use a wide-int here and defer the tree allocation to the result
wide_int wi = wi::from (@2, TYPE_PRECISION (inner_type),
TYPE_SIGN (inner_type));
+ wide_int wmin0, wmax0;
+ wide_int w1 = cst_inner;
+ signop sgn = TYPE_SIGN (inner_type);
+ vr = get_range_info (@0, &wmin0, &wmax0);
+
+ if (vr == VR_RANGE)
+ {
+ bool min_ovf;
+ wi::add (wmin0, w1, sgn, &min_ovf);
+
+ bool max_ovf;
+ wi::add (wmax0, w1, sgn, &max_ovf);
So I guess we never run into the outer_op == minus case as the above is
clearly wrong for that?
The comment above says "if there is no overflow" but below you allow
min_ovf && max_ovf without any further explanation.
+ wrapped_range = (min_ovf && !max_ovf) || (!min_ovf && max_ovf);
+ }
+ }
+ }
+ (if (cst_inner && !wrapped_range)
+ (convert (outer_op @0 { cst_inner; })))
thus
(if ((min_ovf && !max_ovf) || ....)
(convert (outer_op @0 { wide_int_to_tree (inner_type, w1); } ))))
try to keep vertical spacing in patterns minimal -- I belive that patterns
should be small enough to fit in a terminal window (24 lines).
Richard.
+ ))))
+#endif
> Regards
> Robin
next prev parent reply other threads:[~2017-06-27 12:14 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-21 10:42 [PATCH] Tree-level fix for PR 69526 Robin Dapp
2016-07-21 11:28 ` Richard Biener
2016-08-22 14:58 ` Robin Dapp
2016-09-05 7:50 ` Robin Dapp
2016-09-14 13:08 ` Richard Biener
2016-09-14 17:04 ` Jeff Law
2016-10-14 8:33 ` Robin Dapp
2016-09-20 12:39 ` Robin Dapp
2016-09-20 15:31 ` Jeff Law
2016-10-05 10:40 ` Robin Dapp
2016-10-14 11:49 ` Richard Biener
2016-11-16 15:54 ` Robin Dapp
2016-11-25 6:49 ` Robin Dapp
2016-11-25 13:47 ` Richard Biener
2016-11-28 11:13 ` Richard Biener
2016-11-28 13:26 ` Robin Dapp
2016-12-05 7:57 ` Robin Dapp
2016-12-06 13:03 ` Richard Biener
2016-12-07 16:15 ` Robin Dapp
2016-12-13 14:13 ` Richard Biener
2017-01-10 13:33 ` Robin Dapp
2017-01-17 7:34 ` Robin Dapp
2017-01-17 9:48 ` Richard Biener
2017-02-02 9:27 ` Robin Dapp
2017-05-09 7:31 ` Robin Dapp
2017-05-11 15:08 ` Bin.Cheng
2017-05-18 14:47 ` Robin Dapp
2017-05-18 14:48 ` [PATCH 1/3] Simplify wrapped binops Robin Dapp
2017-05-18 14:49 ` [PATCH 2/3] " Robin Dapp
2017-05-18 15:46 ` Bin.Cheng
2017-05-18 16:09 ` Robin Dapp
2017-05-18 17:15 ` Bin.Cheng
2017-05-19 10:13 ` Robin Dapp
2017-05-19 10:22 ` Bin.Cheng
2017-05-19 10:32 ` Richard Biener
2017-06-20 13:08 ` Robin Dapp
2017-06-20 13:49 ` Richard Biener
2017-06-21 11:44 ` Robin Dapp
2017-06-27 7:17 ` Robin Dapp
2017-06-27 12:14 ` Richard Biener [this message]
2017-06-28 14:35 ` Robin Dapp
2017-07-03 13:10 ` Richard Biener
2017-07-05 8:51 ` Robin Dapp
2017-07-05 8:54 ` Robin Dapp
2017-07-15 9:58 ` Marc Glisse
2017-05-18 15:08 ` [PATCH 3/3] " Robin Dapp
2016-08-23 7:11 ` [PATCH] Tree-level fix for PR 69526 Robin Dapp
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='CAFiYyc0mkd=OQzJycfA42BWqkdb1XA8iBRS1KOSCUXbKkn=DTg@mail.gmail.com' \
--to=richard.guenther@gmail.com \
--cc=amker.cheng@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=rdapp@linux.vnet.ibm.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).