This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GCC][PATCH] Enable underflow check in canonicalize_comparison. (PR86995)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]