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
