public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <rdsandiford@googlemail.com>
To: gcc-patches@gcc.gnu.org
Cc: zadeck@naturalbridge.com, mikestump@comcast.net
Subject: [wide-int] Stricter type checking in wide_int constructor
Date: Thu, 24 Apr 2014 22:07:00 -0000	[thread overview]
Message-ID: <87vbtye61q.fsf@talisman.default> (raw)

At the moment we prohibit "widest_int = wide_int" and "offset_int = wide_int".
These would be correct only if the wide_int had the same precision as
widest_int and offset_int respectively, but since those precisions
don't really correspond to a particular language-level precision,
such cases should never occur in practice.

We also prohibit "wide_int = HOST_WIDE_INT" on the basis that the wide_int
would naturally get the precision of HOST_WIDE_INT, which is a host property.

However, we allowed "wide_int = widest_int" and "wide_int = offset_int".
This is always safe in the sense that, unlike "widest_int = wide_int"
and "offset_int = wide_int", they would never trigger an assertion
failure in themselves.  But I think in practice they're always going to
be a mistake.  The only arithmetic you can do on the resulting wide_int
is with wide_ints that have been assigned in the same way, in which case
you should be doing the arithmetic on widest_int or offset_int instead.
And if you don't want to do arithmetic, but simply want to access the value,
you should use wide_int_ref instead.  This avoids unnecessary copying.

This patch adds an extra STATIC_ASSERT to trap that case and fixes
the minor fallout.  The to_mpz change contains another fix: we applied
small_prec without checking whether len was the maximum value.
Also, it seems safer to use alloca now that we have the extra-wide
integer in vrp.

Putting the STATIC_ASSERTs in their own scope is a bit clunky, but a lot
of this code would be much cleaner with C++11...

Tested on x86_64-linux-gnu and included in the asm comparison.  OK to install?

Thanks,
Richard


Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2014-04-24 08:30:16.191670326 +0100
+++ gcc/emit-rtl.c	2014-04-24 08:30:19.117694968 +0100
@@ -535,7 +535,7 @@ lookup_const_wide_int (rtx wint)
    (if TARGET_SUPPORTS_WIDE_INT).  */
 
 rtx
-immed_wide_int_const (const wide_int &v, enum machine_mode mode)
+immed_wide_int_const (const wide_int_ref &v, enum machine_mode mode)
 {
   unsigned int len = v.get_len ();
   unsigned int prec = GET_MODE_PRECISION (mode);
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	2014-04-24 08:30:16.191670326 +0100
+++ gcc/rtl.h	2014-04-24 08:30:19.117694968 +0100
@@ -2008,7 +2008,7 @@ extern double_int rtx_to_double_int (con
 #endif
 extern void cwi_output_hex (FILE *, const_rtx);
 #ifndef GENERATOR_FILE
-extern rtx immed_wide_int_const (const wide_int &cst, enum machine_mode mode);
+extern rtx immed_wide_int_const (const wide_int_ref &, enum machine_mode);
 #endif
 #if TARGET_SUPPORTS_WIDE_INT == 0
 extern rtx immed_double_const (HOST_WIDE_INT, HOST_WIDE_INT,
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c	2014-04-24 08:30:16.191670326 +0100
+++ gcc/tree-ssa-ccp.c	2014-04-24 08:30:19.118694976 +0100
@@ -218,7 +218,8 @@ dump_lattice_value (FILE *outf, const ch
 	}
       else
 	{
-	  wide_int cval = wi::bit_and_not (wi::to_widest (val.value), val.mask);
+	  widest_int cval = wi::bit_and_not (wi::to_widest (val.value),
+					     val.mask);
 	  fprintf (outf, "%sCONSTANT ", prefix);
 	  print_hex (cval, outf);
 	  fprintf (outf, " (");
@@ -1249,7 +1250,7 @@ bit_value_binop_1 (enum tree_code code,
     case RROTATE_EXPR:
       if (r2mask == 0)
 	{
-	  wide_int shift = r2val;
+	  widest_int shift = r2val;
 	  if (shift == 0)
 	    {
 	      *mask = r1mask;
@@ -1286,7 +1287,7 @@ bit_value_binop_1 (enum tree_code code,
 	 is zero.  */
       if (r2mask == 0)
 	{
-	  wide_int shift = r2val;
+	  widest_int shift = r2val;
 	  if (shift == 0)
 	    {
 	      *mask = r1mask;
Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c	2014-04-24 08:30:16.191670326 +0100
+++ gcc/tree-vrp.c	2014-04-24 08:30:19.119694985 +0100
@@ -3860,7 +3860,8 @@ adjust_range_with_scev (value_range_t *v
 	  signop sgn = TYPE_SIGN (TREE_TYPE (step));
 	  bool overflow;
 
-	  wide_int wtmp = wi::mul (wi::to_widest (step), nit, sgn, &overflow);
+	  widest_int wtmp = wi::mul (wi::to_widest (step), nit, sgn,
+				     &overflow);
 	  /* If the multiplication overflowed we can't do a meaningful
 	     adjustment.  Likewise if the result doesn't fit in the type
 	     of the induction variable.  For a signed type we have to
Index: gcc/wide-int-print.cc
===================================================================
--- gcc/wide-int-print.cc	2014-04-24 08:30:16.191670326 +0100
+++ gcc/wide-int-print.cc	2014-04-24 08:30:19.120694993 +0100
@@ -34,7 +34,7 @@ along with GCC; see the file COPYING3.
   (((PREC) + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT)
 
 void
-print_dec (const wide_int &wi, char *buf, signop sgn)
+print_dec (const wide_int_ref &wi, char *buf, signop sgn)
 {
   if (sgn == SIGNED)
     print_decs (wi, buf);
@@ -43,7 +43,7 @@ print_dec (const wide_int &wi, char *buf
 }
 
 void
-print_dec (const wide_int &wi, FILE *file, signop sgn)
+print_dec (const wide_int_ref &wi, FILE *file, signop sgn)
 {
   if (sgn == SIGNED)
     print_decs (wi, file);
@@ -56,7 +56,7 @@ print_dec (const wide_int &wi, FILE *fil
    in a HWI.  Other print in hex.  */
 
 void
-print_decs (const wide_int &wi, char *buf)
+print_decs (const wide_int_ref &wi, char *buf)
 {
   if ((wi.get_precision () <= HOST_BITS_PER_WIDE_INT)
       || (wi.get_len () == 1))
@@ -74,7 +74,7 @@ print_decs (const wide_int &wi, char *bu
    in a HWI.  Other print in hex.  */
 
 void
-print_decs (const wide_int &wi, FILE *file)
+print_decs (const wide_int_ref &wi, FILE *file)
 {
   char buf[WIDE_INT_PRINT_BUFFER_SIZE];
   print_decs (wi, buf);
@@ -85,7 +85,7 @@ print_decs (const wide_int &wi, FILE *fi
    in a HWI.  Other print in hex.  */
 
 void
-print_decu (const wide_int &wi, char *buf)
+print_decu (const wide_int_ref &wi, char *buf)
 {
   if ((wi.get_precision () <= HOST_BITS_PER_WIDE_INT)
       || (wi.get_len () == 1 && !wi::neg_p (wi)))
@@ -98,7 +98,7 @@ print_decu (const wide_int &wi, char *bu
    in a HWI.  Other print in hex.  */
 
 void
-print_decu (const wide_int &wi, FILE *file)
+print_decu (const wide_int_ref &wi, FILE *file)
 {
   char buf[WIDE_INT_PRINT_BUFFER_SIZE];
   print_decu (wi, buf);
@@ -106,7 +106,7 @@ print_decu (const wide_int &wi, FILE *fi
 }
 
 void
-print_hex (const wide_int &wi, char *buf)
+print_hex (const wide_int_ref &wi, char *buf)
 {
   int i = wi.get_len ();
 
@@ -136,7 +136,7 @@ print_hex (const wide_int &wi, char *buf
 /* Print one big hex number to FILE.  Note that some assemblers may not
    accept this for large modes.  */
 void
-print_hex (const wide_int &wi, FILE *file)
+print_hex (const wide_int_ref &wi, FILE *file)
 {
   char buf[WIDE_INT_PRINT_BUFFER_SIZE];
   print_hex (wi, buf);
Index: gcc/wide-int-print.h
===================================================================
--- gcc/wide-int-print.h	2014-04-24 08:30:16.191670326 +0100
+++ gcc/wide-int-print.h	2014-04-24 08:30:19.120694993 +0100
@@ -27,13 +27,13 @@ #define WIDE_INT_PRINT_BUFFER_SIZE (WIDE
 
 /* Printing functions.  */
 
-extern void print_dec (const wide_int &wi, char *buf, signop sgn);
-extern void print_dec (const wide_int &wi, FILE *file, signop sgn);
-extern void print_decs (const wide_int &wi, char *buf);
-extern void print_decs (const wide_int &wi, FILE *file);
-extern void print_decu (const wide_int &wi, char *buf);
-extern void print_decu (const wide_int &wi, FILE *file);
-extern void print_hex (const wide_int &wi, char *buf);
-extern void print_hex (const wide_int &wi, FILE *file);
+extern void print_dec (const wide_int_ref &wi, char *buf, signop sgn);
+extern void print_dec (const wide_int_ref &wi, FILE *file, signop sgn);
+extern void print_decs (const wide_int_ref &wi, char *buf);
+extern void print_decs (const wide_int_ref &wi, FILE *file);
+extern void print_decu (const wide_int_ref &wi, char *buf);
+extern void print_decu (const wide_int_ref &wi, FILE *file);
+extern void print_hex (const wide_int_ref &wi, char *buf);
+extern void print_hex (const wide_int_ref &wi, FILE *file);
 
 #endif /* WIDE_INT_PRINT_H */
Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc	2014-04-24 08:30:16.191670326 +0100
+++ gcc/wide-int.cc	2014-04-24 09:51:08.056774754 +0100
@@ -180,37 +180,36 @@ wi::from_buffer (const unsigned char *bu
   return result;
 }
 
-/* Sets RESULT from THIS, the sign is taken according to SGN.  */
+/* Sets RESULT from X, the sign is taken according to SGN.  */
 void
-wi::to_mpz (wide_int x, mpz_t result, signop sgn)
+wi::to_mpz (const wide_int_ref &x, mpz_t result, signop sgn)
 {
-  bool negative = false;
   int len = x.get_len ();
   const HOST_WIDE_INT *v = x.get_val ();
-  int small_prec = x.get_precision () & (HOST_BITS_PER_WIDE_INT - 1);
+  int excess = len * HOST_BITS_PER_WIDE_INT - x.get_precision ();
 
   if (wi::neg_p (x, sgn))
     {
-      negative = true;
       /* We use ones complement to avoid -x80..0 edge case that -
 	 won't work on.  */
-      x = ~x;
+      HOST_WIDE_INT *t = XALLOCAVEC (HOST_WIDE_INT, len);
+      for (int i = 0; i < len; i++)
+	t[i] = ~v[i];
+      if (excess > 0)
+	t[len - 1] = (unsigned HOST_WIDE_INT) t[len - 1] << excess >> excess;
+      mpz_import (result, len, -1, sizeof (HOST_WIDE_INT), 0, 0, t);
+      mpz_com (result, result);
     }
-
-  if (sgn == UNSIGNED && small_prec)
+  else if (excess > 0)
     {
-      HOST_WIDE_INT t[WIDE_INT_MAX_ELTS];
-
+      HOST_WIDE_INT *t = XALLOCAVEC (HOST_WIDE_INT, len);
       for (int i = 0; i < len - 1; i++)
 	t[i] = v[i];
-      t[len-1] = zext_hwi (v[len-1], small_prec);
+      t[len - 1] = (unsigned HOST_WIDE_INT) v[len - 1] << excess >> excess;
       mpz_import (result, len, -1, sizeof (HOST_WIDE_INT), 0, 0, t);
     }
   else
     mpz_import (result, len, -1, sizeof (HOST_WIDE_INT), 0, 0, v);
-
-  if (negative)
-    mpz_com (result, result);
 }
 
 /* Returns VAL converted to TYPE.  If WRAP is true, then out-of-range
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h	2014-04-24 08:30:16.191670326 +0100
+++ gcc/wide-int.h	2014-04-24 09:49:51.783075748 +0100
@@ -1018,7 +1018,8 @@ inline wide_int_storage::wide_int_storag
 template <typename T>
 inline wide_int_storage::wide_int_storage (const T &x)
 {
-  STATIC_ASSERT (!wi::int_traits<T>::host_dependent_precision);
+  { STATIC_ASSERT (!wi::int_traits<T>::host_dependent_precision); }
+  { STATIC_ASSERT (wi::int_traits<T>::precision_type != wi::CONST_PRECISION); }
   WIDE_INT_REF_FOR (T) xi (x);
   precision = xi.precision;
   wi::copy (*this, xi);
@@ -3073,7 +3074,7 @@ gt_pch_nx (trailing_wide_ints <N> *, voi
   wide_int from_buffer (const unsigned char *, unsigned int);
 
 #ifndef GENERATOR_FILE
-  void to_mpz (wide_int, mpz_t, signop);
+  void to_mpz (const wide_int_ref &, mpz_t, signop);
 #endif
 
   wide_int mask (unsigned int, bool, unsigned int);



             reply	other threads:[~2014-04-24 22:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-24 22:07 Richard Sandiford [this message]
2014-04-28  9:38 ` Richard Sandiford
2014-04-28 16:26   ` Mike Stump
2014-04-28 16:48     ` Kenneth Zadeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87vbtye61q.fsf@talisman.default \
    --to=rdsandiford@googlemail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=mikestump@comcast.net \
    --cc=zadeck@naturalbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).