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?

2017-10-06  Jakub Jelinek  <ja...@redhat.com>

        PR c/82437
        * c-warn.c (warn_tautological_bitwise_comparison): Use wi::to_wide
        instead of wide_int::from.

        * c-c++-common/Wtautological-compare-7.c: New test.

--- gcc/c-family/c-warn.c.jj    2017-10-06 09:05:41.000000000 +0200
+++ gcc/c-family/c-warn.c       2017-10-06 09:28:41.597944319 +0200
@@ -362,8 +362,8 @@ warn_tautological_bitwise_comparison (lo
   int prec = MAX (TYPE_PRECISION (TREE_TYPE (cst)),
                  TYPE_PRECISION (TREE_TYPE (bitopcst)));
 
-  wide_int bitopcstw = wide_int::from (bitopcst, prec, UNSIGNED);
-  wide_int cstw = wide_int::from (cst, prec, UNSIGNED);
+  wide_int bitopcstw = wi::to_wide (bitopcst, prec);
+  wide_int cstw = wi::to_wide (cst, prec);
 
   wide_int res;
   if (TREE_CODE (bitop) == BIT_AND_EXPR)
--- gcc/testsuite/c-c++-common/Wtautological-compare-7.c.jj     2017-10-06 
09:26:04.812942462 +0200
+++ gcc/testsuite/c-c++-common/Wtautological-compare-7.c        2017-10-06 
09:25:57.545035088 +0200
@@ -0,0 +1,11 @@
+/* PR c/82437 */
+/* { dg-do compile { target int32 } } */
+/* { dg-options "-Wtautological-compare" } */
+
+int
+foo (unsigned long long int x)
+{
+  if ((x | 0x190000000ULL) != -1879048192)     /* { dg-bogus "bitwise 
comparison always evaluates to" } */
+    return 0;
+  return 1;
+}


        Jakub

Reply via email to