Re: Some wide-int review comments

[Kenneth Zadeck]

Committed with all your changes as revision 205049.

the only interesting change is the response to your last comment.

I changed the frag to:

   /* Third, unsigned integers with top bit set never fit signed types.  */
-  if (!TYPE_UNSIGNED (type) && sgn_c == UNSIGNED && wi::neg_p (c))
-    return false;
+  if (!TYPE_UNSIGNED (type) && sgn_c == UNSIGNED)
+    {
+      int uprec = GET_MODE_PRECISION (TYPE_MODE TREE_TYPE (c));
+      if (uprec < TYPE_PRECISION (TREE_TYPE (c)))
+    {
+      /* When a tree_cst is converted to a wide-int, the precision
+         is taken from the type.  However, if the precision of the
+         mode underneath the type is smaller than that, it is
+         possible that the value will not fit.  The test below
+         fails if any bit is set between the sign bit of the
+         underlying mode and the top bit of the type.  */
+      if (wi::ne_p (wi::zext (c, uprec - 1), c))
+        return false;
+    }
+      else if (wi::neg_p (c))
+    return false;
+    }


On 11/18/2013 04:29 AM, Richard Sandiford wrote:

Thanks for the changes.

@@ -8162,7 +8162,7 @@ fold_builtin_logarithm (location_t loc,
            /* Prepare to do logN(exp10(exponent) -> exponent*logN(10).  */
            {
              REAL_VALUE_TYPE dconst10;
-             real_from_integer (&dconst10, VOIDmode, 10, SIGNED);
+             real_from_integer (&dconst10, VOIDmode, wi::shwi (10, 32), 
SIGNED);
              x = build_real (type, dconst10);
            }
            exponent = CALL_EXPR_ARG (arg, 0);
@@ -8315,7 +8315,7 @@ fold_builtin_pow (location_t loc, tree f

/* Check for an integer exponent. */

        n = real_to_integer (&c);
-      real_from_integer (&cint, VOIDmode, n, SIGNED);
+      real_from_integer (&cint, VOIDmode, wi::shwi (n, 
HOST_BITS_PER_WIDE_INT), SIGNED);
        if (real_identical (&c, &cint))
        {
          /* Attempt to evaluate pow at compile-time, unless this should

Are these changes necessary?  The original calls ought to work as-is,
since the function takes a const wide_int_ref &.  Same for the rest of
the patch.

Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c    (revision 204918)
+++ gcc/c/c-parser.c    (working copy)
@@ -13375,7 +13375,7 @@ c_parser_cilk_clause_vectorlength (c_par
        || !TREE_CONSTANT (expr)
        || !INTEGRAL_TYPE_P (TREE_TYPE (expr)))
      error_at (loc, "vectorlength must be an integer constant");
-  else if (exact_log2 (tree_to_hwi (expr)) == -1)
+  else if (wi::eq_p (wi::exact_log2 (expr), -1))
      error_at (loc, "vectorlength must be a power of 2");
    else
      {

FWIW:

   wi::exact_log2 (expr) == -1

should still work.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c     (revision 204918)
+++ gcc/dwarf2out.c     (working copy)
@@ -13428,8 +13428,6 @@ loc_descriptor (rtx rtl, enum machine_mo

if (mode != VOIDmode && (dwarf_version >= 4 || !dwarf_strict))

        {
-         gcc_assert (mode == GET_MODE (rtl) || VOIDmode == GET_MODE (rtl));
-
          /* Note that a CONST_DOUBLE rtx could represent either an integer
             or a floating-point constant.  A CONST_DOUBLE is used whenever
             the constant requires more than one word in order to be

The copy of the CONST_DOUBLE comment is still there though.
(This is handling CONST_WIDE_INT rather than CONST_DOUBLE.)

Index: gcc/tree.c
===================================================================
--- gcc/tree.c  (revision 204918)
+++ gcc/tree.c  (working copy)
@@ -8536,8 +8536,18 @@ retry:
      return true;

/* Third, unsigned integers with top bit set never fit signed types. */

-  if (!TYPE_UNSIGNED (type) && sgn_c == UNSIGNED && wi::neg_p (c))
-    return false;
+  if (!TYPE_UNSIGNED (type) && sgn_c == UNSIGNED)
+    {
+      int uprec = GET_MODE_PRECISION (TYPE_MODE TREE_TYPE (c));
+      if (uprec < TYPE_PRECISION (TREE_TYPE (c)))
+       {
+         wide_int x = wi::sext (c, uprec);
+         if (wi::neg_p (x) || wi::ne_p (x, c))
+           return false;
+       }
+      else if (wi::neg_p (c))
+       return false;
+    }

/* If we haven't been able to decide at this point, there nothing more we

       can check ourselves here.  Look at the base type if we have one and it

I don't really understand this change, but I suppose it's part of
the trunk patch.

Looks good to me otherwise FWIW.

Thanks,
Richard