This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C PATCH] Fix -Wtautological-compare (PR c/82437; #2)
- From: Marek Polacek <polacek at redhat dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: "Joseph S. Myers" <joseph at codesourcery dot com>, gcc-patches at gcc dot gnu dot org
- Date: Mon, 9 Oct 2017 15:39:57 +0200
- Subject: Re: [C PATCH] Fix -Wtautological-compare (PR c/82437; #2)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=polacek at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 476ED5D68C
- References: <20171005203426.GJ18588@tucnak> <20171005210557.GO7740@redhat.com> <20171006122141.GP18588@tucnak>
On Fri, Oct 06, 2017 at 02:21:41PM +0200, Jakub Jelinek wrote:
> On Thu, Oct 05, 2017 at 11:05:57PM +0200, Marek Polacek wrote:
> > On Thu, Oct 05, 2017 at 10:34:26PM +0200, Jakub Jelinek wrote:
> > > Hi!
> > >
> > > In warn_tautological_bitwise_comparison, there is even a comment
> > > mentioning the fact that the types of the two constants might not be the
> > > same (it is called with the original arguments before they are promoted
> > > to common type for the comparison).
> > >
> > > On the following testcase, one of the constants was unsigned (because
> > > it has been converted to that when anded with unsigned variable), while
> > > the other was signed and wi::to_widest (bitopcst) & wi::to_widest (cst)
> > > wasn't sign-extended from 32-bit (because bitopcst was unsigned),
> > > while wi::to_widest (cst) was (because cst was int), so we warned even
> > > when we shouldn't.
> > >
> > > The following patch instead uses the precision of the larger type and
> > > zero extends from that (because we really don't need to care about bits
> > > beyond that precision).
> > >
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > Looks ok, thanks.
>
> Actually, after committing it I've realized it was wrong. Sorry for that.
>
> By forcing UNSIGNED sign in wide_int::from it forces zero extension from the
> precision of the tree to prec, but if there are any bits in between, we
> need extension according to the sign of the constant. As we create the
> wide_int with prec, comparison of the wide_ints only looks at the low prec
> bits (or, if both wide_ints are guaranteed to be sign-extended from that
> the bits above will match too).
> So, the right thing to do is wide_int::from (cst, prec, TYPE_SIGN (TREE_TYPE (cst)))
> which is wi::to_wide (cst, prec).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Marek