This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [GCC][PATCH] Enable underflow check in canonicalize_comparison. (PR86995)
- From: Vlad Lazar <vlad dot lazar at arm dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>, Jeff Law <law at redhat dot com>, richard dot sandiford at arm dot com
- Date: Thu, 30 Aug 2018 10:34:44 +0100
- Subject: Re: [GCC][PATCH] Enable underflow check in canonicalize_comparison. (PR86995)
- References: <5B86CC3E.40006@arm.com> <20180829164351.GE2218@tucnak> <5B86CE7E.3040104@arm.com> <20180829165522.GF2218@tucnak>
On 29/08/18 17:55, Jakub Jelinek wrote:
On Wed, Aug 29, 2018 at 05:49:02PM +0100, Vlad Lazar wrote:
On 29/08/18 17:43, Jakub Jelinek wrote:
On Wed, Aug 29, 2018 at 05:39:26PM +0100, Vlad Lazar wrote:
r263591 introduced the following regressions on multiple platforms:
+FAIL: c-c++-common/torture/builtin-arith-overflow-17.c -O0 execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-17.c -O2 execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-17.c -O2 -flto execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-17.c -O2 -flto -flto-partition=none execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-p-17.c -O0 execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-p-17.c -O2 execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-p-17.c -O2 -flto execution test
+FAIL: c-c++-common/torture/builtin-arith-overflow-p-17.c -O2 -flto -flto-partition=none execution test
The cause for this was that in canonicalize_comparison wi::add was used to add a negative number. This meant
that in case of underflow the flag would not have been set. The solution is to use wi::sub if the immediate
needs to be decremented.
The patch fixes the mentioned regressions.
Bootstrapped and regtested on aarch64-none-linux-gnu and x86_64-pc-linux-gnu.
LGTM, but ChangeLog entry is missing, can you please provide one before I
can ack it?
Sorry about that. Here's the ChangeLog:
gcc/
2018-08-29 Vlad Lazar <vlad.lazar@arm.com>
* expmed.c (canonicalize_comparison): Enable underflow check.
There should be empty line after the date/name/email line.
For ChangeLog entries of GCC bugzilla PR bugfixes then the next line
should be
PR middle-end/86995
and only then the expmed.c line. "Enable underflow check." doesn't look
sufficiently descriptive to what it does, I'd use
* expmed.c (canonicalize_comparison): Use wi::sub instead of wi::add
if to_add is negative.
Ok for trunk with those changes.
Jakub
Thanks for the swift review.
I've attached the committed patch.
Thanks,
Vlad
Index: ChangeLog
===================================================================
--- ChangeLog (revision 263972)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2018-08-30 Vlad Lazar <vlad.lazar@arm.com>
+
+ PR middle-end/86995
+ * expmed.c (canonicalize_comparison): Use wi::sub instead of wi::add
+ if to_add is negative.
+
2018-08-29 Bernd Edlinger <bernd.edlinger@hotmail.de>
PR middle-end/87053
Index: expmed.c
===================================================================
--- expmed.c (revision 263972)
+++ expmed.c (working copy)
@@ -6239,7 +6239,13 @@
wrapping around in the case of unsigned values. If any occur
cancel the optimization. */
wi::overflow_type overflow = wi::OVF_NONE;
- wide_int imm_modif = wi::add (imm_val, to_add, sgn, &overflow);
+ wide_int imm_modif;
+
+ if (to_add == 1)
+ imm_modif = wi::add (imm_val, 1, sgn, &overflow);
+ else
+ imm_modif = wi::sub (imm_val, 1, sgn, &overflow);
+
if (overflow)
return;