Re: [wide-int] Stricter type checking in wide_int constructor

[Richard Sandiford <rsandifo at linux dot vnet dot ibm dot com>]

Ping.  FWIW this is the last patch I have lined up before the merge.
I repeated the asm comparison test I did a few months ago on one target
per config/ architecture and there were no unexpected changes.

Richard Sandiford <rdsandiford@googlemail.com> writes:
> 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);