public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Richard Sandiford <richard.sandiford@arm.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] wide-int: Remove rwide_int, introduce dw_wide_int
Date: Tue, 10 Oct 2023 09:30:31 +0000 (UTC)	[thread overview]
Message-ID: <nycvar.YFH.7.77.849.2310100930140.5561@jbgna.fhfr.qr> (raw)
In-Reply-To: <ZSQVTcoN+gxgCJCF@tucnak>

On Mon, 9 Oct 2023, Jakub Jelinek wrote:

> On Mon, Oct 09, 2023 at 12:55:02PM +0200, Jakub Jelinek wrote:
> > This makes wide_int unusable in GC structures, so for dwarf2out
> > which was the only place which needed it there is a new rwide_int type
> > (restricted wide_int) which supports only up to RWIDE_INT_MAX_ELTS limbs
> > inline and is trivially copyable (dwarf2out should never deal with large
> > _BitInt constants, those should have been lowered earlier).
> 
> As discussed on IRC, the dwarf2out.{h,cc} needs are actually quite limited,
> it just needs to allocate new GC structures val_wide points to (constructed
> from some const wide_int_ref &) and needs to call operator==,
> get_precision, elt, get_len and get_val methods on it.
> Even trailing_wide_int would be overkill for that, the following just adds
> a new struct with precision/len and trailing val array members and
> implements the needed methods (only 2 of them using wide_int_ref constructed
> from those).
> 
> Incremental patch, so far compile time tested only:

LGTM, wonder if we can push this separately as prerequesite?

Thanks,
Richard.

> --- gcc/wide-int.h.jj	2023-10-09 14:37:45.878940132 +0200
> +++ gcc/wide-int.h	2023-10-09 16:06:39.326805176 +0200
> @@ -27,7 +27,7 @@ along with GCC; see the file COPYING3.
>     other longer storage GCC representations (rtl and tree).
>  
>     The actual precision of a wide_int depends on the flavor.  There
> -   are four predefined flavors:
> +   are three predefined flavors:
>  
>       1) wide_int (the default).  This flavor does the math in the
>       precision of its input arguments.  It is assumed (and checked)
> @@ -80,12 +80,7 @@ along with GCC; see the file COPYING3.
>         wi::leu_p (a, b) as a more efficient short-hand for
>         "a >= 0 && a <= b". ]
>  
> -     3) rwide_int.  Restricted wide_int.  This is similar to
> -     wide_int, but maximum possible precision is RWIDE_INT_MAX_PRECISION
> -     and it always uses an inline buffer.  offset_int and rwide_int are
> -     GC-friendly, wide_int and widest_int are not.
> -
> -     4) widest_int.  This representation is an approximation of
> +     3) widest_int.  This representation is an approximation of
>       infinite precision math.  However, it is not really infinite
>       precision math as in the GMP library.  It is really finite
>       precision math where the precision is WIDEST_INT_MAX_PRECISION.
> @@ -257,9 +252,6 @@ along with GCC; see the file COPYING3.
>  #define WIDE_INT_MAX_ELTS 255
>  #define WIDE_INT_MAX_PRECISION (WIDE_INT_MAX_ELTS * HOST_BITS_PER_WIDE_INT)
>  
> -#define RWIDE_INT_MAX_ELTS WIDE_INT_MAX_INL_ELTS
> -#define RWIDE_INT_MAX_PRECISION WIDE_INT_MAX_INL_PRECISION
> -
>  /* Precision of widest_int and largest _BitInt precision + 1 we can
>     support.  */
>  #define WIDEST_INT_MAX_ELTS 510
> @@ -343,7 +335,6 @@ STATIC_ASSERT (WIDE_INT_MAX_INL_ELTS < W
>  template <typename T> class generic_wide_int;
>  template <int N> class fixed_wide_int_storage;
>  class wide_int_storage;
> -class rwide_int_storage;
>  template <int N> class widest_int_storage;
>  
>  /* An N-bit integer.  Until we can use typedef templates, use this instead.  */
> @@ -352,7 +343,6 @@ template <int N> class widest_int_storag
>  
>  typedef generic_wide_int <wide_int_storage> wide_int;
>  typedef FIXED_WIDE_INT (ADDR_MAX_PRECISION) offset_int;
> -typedef generic_wide_int <rwide_int_storage> rwide_int;
>  typedef generic_wide_int <widest_int_storage <WIDE_INT_MAX_INL_PRECISION> > widest_int;
>  typedef generic_wide_int <widest_int_storage <WIDE_INT_MAX_INL_PRECISION * 2> > widest2_int;
>  
> @@ -1371,180 +1361,6 @@ wi::int_traits <wide_int_storage>::get_b
>      return wi::get_precision (x);
>  }
>  
> -/* The storage used by rwide_int.  */
> -class GTY(()) rwide_int_storage
> -{
> -private:
> -  HOST_WIDE_INT val[RWIDE_INT_MAX_ELTS];
> -  unsigned int len;
> -  unsigned int precision;
> -
> -public:
> -  rwide_int_storage () = default;
> -  template <typename T>
> -  rwide_int_storage (const T &);
> -
> -  /* The standard generic_rwide_int storage methods.  */
> -  unsigned int get_precision () const;
> -  const HOST_WIDE_INT *get_val () const;
> -  unsigned int get_len () const;
> -  HOST_WIDE_INT *write_val (unsigned int);
> -  void set_len (unsigned int, bool = false);
> -
> -  template <typename T>
> -  rwide_int_storage &operator = (const T &);
> -
> -  static rwide_int from (const wide_int_ref &, unsigned int, signop);
> -  static rwide_int from_array (const HOST_WIDE_INT *, unsigned int,
> -			       unsigned int, bool = true);
> -  static rwide_int create (unsigned int);
> -};
> -
> -namespace wi
> -{
> -  template <>
> -  struct int_traits <rwide_int_storage>
> -  {
> -    static const enum precision_type precision_type = VAR_PRECISION;
> -    /* Guaranteed by a static assert in the rwide_int_storage constructor.  */
> -    static const bool host_dependent_precision = false;
> -    static const bool is_sign_extended = true;
> -    static const bool needs_write_val_arg = false;
> -    template <typename T1, typename T2>
> -    static rwide_int get_binary_result (const T1 &, const T2 &);
> -    template <typename T1, typename T2>
> -    static unsigned int get_binary_precision (const T1 &, const T2 &);
> -  };
> -}
> -
> -/* Initialize the storage from integer X, in its natural precision.
> -   Note that we do not allow integers with host-dependent precision
> -   to become rwide_ints; rwide_ints must always be logically independent
> -   of the host.  */
> -template <typename T>
> -inline rwide_int_storage::rwide_int_storage (const T &x)
> -{
> -  STATIC_ASSERT (!wi::int_traits<T>::host_dependent_precision);
> -  STATIC_ASSERT (wi::int_traits<T>::precision_type != wi::CONST_PRECISION);
> -  STATIC_ASSERT (wi::int_traits<T>::precision_type
> -		 != wi::WIDEST_CONST_PRECISION);
> -  WIDE_INT_REF_FOR (T) xi (x);
> -  precision = xi.precision;
> -  gcc_assert (precision <= RWIDE_INT_MAX_PRECISION);
> -  wi::copy (*this, xi);
> -}
> -
> -template <typename T>
> -inline rwide_int_storage&
> -rwide_int_storage::operator = (const T &x)
> -{
> -  STATIC_ASSERT (!wi::int_traits<T>::host_dependent_precision);
> -  STATIC_ASSERT (wi::int_traits<T>::precision_type != wi::CONST_PRECISION);
> -  STATIC_ASSERT (wi::int_traits<T>::precision_type
> -		 != wi::WIDEST_CONST_PRECISION);
> -  WIDE_INT_REF_FOR (T) xi (x);
> -  precision = xi.precision;
> -  gcc_assert (precision <= RWIDE_INT_MAX_PRECISION);
> -  wi::copy (*this, xi);
> -  return *this;
> -}
> -
> -inline unsigned int
> -rwide_int_storage::get_precision () const
> -{
> -  return precision;
> -}
> -
> -inline const HOST_WIDE_INT *
> -rwide_int_storage::get_val () const
> -{
> -  return val;
> -}
> -
> -inline unsigned int
> -rwide_int_storage::get_len () const
> -{
> -  return len;
> -}
> -
> -inline HOST_WIDE_INT *
> -rwide_int_storage::write_val (unsigned int)
> -{
> -  return val;
> -}
> -
> -inline void
> -rwide_int_storage::set_len (unsigned int l, bool is_sign_extended)
> -{
> -  len = l;
> -  if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > precision)
> -    val[len - 1] = sext_hwi (val[len - 1],
> -			     precision % HOST_BITS_PER_WIDE_INT);
> -}
> -
> -/* Treat X as having signedness SGN and convert it to a PRECISION-bit
> -   number.  */
> -inline rwide_int
> -rwide_int_storage::from (const wide_int_ref &x, unsigned int precision,
> -			 signop sgn)
> -{
> -  rwide_int result = rwide_int::create (precision);
> -  result.set_len (wi::force_to_size (result.write_val (x.len), x.val, x.len,
> -				     x.precision, precision, sgn));
> -  return result;
> -}
> -
> -/* Create a rwide_int from the explicit block encoding given by VAL and
> -   LEN.  PRECISION is the precision of the integer.  NEED_CANON_P is
> -   true if the encoding may have redundant trailing blocks.  */
> -inline rwide_int
> -rwide_int_storage::from_array (const HOST_WIDE_INT *val, unsigned int len,
> -			       unsigned int precision, bool need_canon_p)
> -{
> -  rwide_int result = rwide_int::create (precision);
> -  result.set_len (wi::from_array (result.write_val (len), val, len, precision,
> -				  need_canon_p));
> -  return result;
> -}
> -
> -/* Return an uninitialized rwide_int with precision PRECISION.  */
> -inline rwide_int
> -rwide_int_storage::create (unsigned int precision)
> -{
> -  rwide_int x;
> -  gcc_assert (precision <= RWIDE_INT_MAX_PRECISION);
> -  x.precision = precision;
> -  return x;
> -}
> -
> -template <typename T1, typename T2>
> -inline rwide_int
> -wi::int_traits <rwide_int_storage>::get_binary_result (const T1 &x,
> -						       const T2 &y)
> -{
> -  /* This shouldn't be used for two flexible-precision inputs.  */
> -  STATIC_ASSERT (wi::int_traits <T1>::precision_type != FLEXIBLE_PRECISION
> -		 || wi::int_traits <T2>::precision_type != FLEXIBLE_PRECISION);
> -  if (wi::int_traits <T1>::precision_type == FLEXIBLE_PRECISION)
> -    return rwide_int::create (wi::get_precision (y));
> -  else
> -    return rwide_int::create (wi::get_precision (x));
> -}
> -
> -template <typename T1, typename T2>
> -inline unsigned int
> -wi::int_traits <rwide_int_storage>::get_binary_precision (const T1 &x,
> -							  const T2 &y)
> -{
> -  /* This shouldn't be used for two flexible-precision inputs.  */
> -  STATIC_ASSERT (wi::int_traits <T1>::precision_type != FLEXIBLE_PRECISION
> -		 || wi::int_traits <T2>::precision_type != FLEXIBLE_PRECISION);
> -  if (wi::int_traits <T1>::precision_type == FLEXIBLE_PRECISION)
> -    return wi::get_precision (y);
> -  else
> -    return wi::get_precision (x);
> -}
> -
>  /* The storage used by FIXED_WIDE_INT (N).  */
>  template <int N>
>  class GTY(()) fixed_wide_int_storage
> @@ -4082,21 +3898,6 @@ void gt_pch_nx (generic_wide_int <wide_i
>  void gt_pch_nx (generic_wide_int <wide_int_storage> *,
>  		gt_pointer_operator, void *) = delete;
>  
> -inline void
> -gt_ggc_mx (generic_wide_int <rwide_int_storage> *)
> -{
> -}
> -
> -inline void
> -gt_pch_nx (generic_wide_int <rwide_int_storage> *)
> -{
> -}
> -
> -inline void
> -gt_pch_nx (generic_wide_int <rwide_int_storage> *, gt_pointer_operator, void *)
> -{
> -}
> -
>  template<int N>
>  void
>  gt_ggc_mx (generic_wide_int <fixed_wide_int_storage <N> > *)
> --- gcc/dwarf2out.h.jj	2023-10-09 14:37:45.890939965 +0200
> +++ gcc/dwarf2out.h	2023-10-09 16:46:14.705816928 +0200
> @@ -30,7 +30,7 @@ typedef struct dw_cfi_node *dw_cfi_ref;
>  typedef struct dw_loc_descr_node *dw_loc_descr_ref;
>  typedef struct dw_loc_list_struct *dw_loc_list_ref;
>  typedef struct dw_discr_list_node *dw_discr_list_ref;
> -typedef rwide_int *rwide_int_ptr;
> +typedef struct dw_wide_int *dw_wide_int_ptr;
>  
>  
>  /* Call frames are described using a sequence of Call Frame
> @@ -252,7 +252,7 @@ struct GTY(()) dw_val_node {
>        unsigned HOST_WIDE_INT
>  	GTY ((tag ("dw_val_class_unsigned_const"))) val_unsigned;
>        double_int GTY ((tag ("dw_val_class_const_double"))) val_double;
> -      rwide_int_ptr GTY ((tag ("dw_val_class_wide_int"))) val_wide;
> +      dw_wide_int_ptr GTY ((tag ("dw_val_class_wide_int"))) val_wide;
>        dw_vec_const GTY ((tag ("dw_val_class_vec"))) val_vec;
>        struct dw_val_die_union
>  	{
> @@ -313,6 +313,35 @@ struct GTY(()) dw_discr_list_node {
>    int dw_discr_range;
>  };
>  
> +struct GTY((variable_size)) dw_wide_int {
> +  unsigned int precision;
> +  unsigned int len;
> +  HOST_WIDE_INT val[1];
> +
> +  unsigned int get_precision () const { return precision; }
> +  unsigned int get_len () const { return len; }
> +  const HOST_WIDE_INT *get_val () const { return val; }
> +  inline HOST_WIDE_INT elt (unsigned int) const;
> +  inline bool operator == (const dw_wide_int &) const;
> +};
> +
> +inline HOST_WIDE_INT
> +dw_wide_int::elt (unsigned int i) const
> +{
> +  if (i < len)
> +    return val[i];
> +  wide_int_ref ref = wi::storage_ref (val, len, precision);
> +  return wi::sign_mask (ref);
> +}
> +
> +inline bool
> +dw_wide_int::operator == (const dw_wide_int &o) const
> +{
> +  wide_int_ref ref1 = wi::storage_ref (val, len, precision);
> +  wide_int_ref ref2 = wi::storage_ref (o.val, o.len, o.precision);
> +  return ref1 == ref2;
> +}
> +
>  /* Interface from dwarf2out.cc to dwarf2cfi.cc.  */
>  extern struct dw_loc_descr_node *build_cfa_loc
>    (dw_cfa_location *, poly_int64);
> --- gcc/dwarf2out.cc.jj	2023-10-09 14:37:45.894939909 +0200
> +++ gcc/dwarf2out.cc	2023-10-09 16:48:24.565014459 +0200
> @@ -397,11 +397,9 @@ dump_struct_debug (tree type, enum debug
>     of the number.  */
>  
>  static unsigned int
> -get_full_len (const rwide_int &op)
> +get_full_len (const dw_wide_int &op)
>  {
> -  int prec = wi::get_precision (op);
> -  return ((prec + HOST_BITS_PER_WIDE_INT - 1)
> -	  / HOST_BITS_PER_WIDE_INT);
> +  return CEIL (op.get_precision (), HOST_BITS_PER_WIDE_INT);
>  }
>  
>  static bool
> @@ -3900,7 +3898,7 @@ static void add_data_member_location_att
>  						struct vlr_context *);
>  static bool add_const_value_attribute (dw_die_ref, machine_mode, rtx);
>  static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *);
> -static void insert_wide_int (const rwide_int &, unsigned char *, int);
> +static void insert_wide_int (const wide_int_ref &, unsigned char *, int);
>  static unsigned insert_float (const_rtx, unsigned char *);
>  static rtx rtl_for_decl_location (tree);
>  static bool add_location_or_const_value_attribute (dw_die_ref, tree, bool);
> @@ -4594,19 +4592,31 @@ AT_unsigned (dw_attr_node *a)
>    return a->dw_attr_val.v.val_unsigned;
>  }
>  
> +dw_wide_int *
> +alloc_dw_wide_int (const wide_int_ref &w)
> +{
> +  dw_wide_int *p
> +    = (dw_wide_int *) ggc_internal_alloc (sizeof (dw_wide_int)
> +					  + ((w.get_len () - 1)
> +					     * sizeof (HOST_WIDE_INT)));
> +  p->precision = w.get_precision ();
> +  p->len = w.get_len ();
> +  memcpy (p->val, w.get_val (), p->len * sizeof (HOST_WIDE_INT));
> +  return p;
> +}
> +
>  /* Add an unsigned wide integer attribute value to a DIE.  */
>  
>  static inline void
>  add_AT_wide (dw_die_ref die, enum dwarf_attribute attr_kind,
> -	     const rwide_int& w)
> +	     const wide_int_ref &w)
>  {
>    dw_attr_node attr;
>  
>    attr.dw_attr = attr_kind;
>    attr.dw_attr_val.val_class = dw_val_class_wide_int;
>    attr.dw_attr_val.val_entry = NULL;
> -  attr.dw_attr_val.v.val_wide = ggc_alloc<rwide_int> ();
> -  *attr.dw_attr_val.v.val_wide = w;
> +  attr.dw_attr_val.v.val_wide = alloc_dw_wide_int (w);
>    add_dwarf_attr (die, &attr);
>  }
>  
> @@ -16714,8 +16724,8 @@ mem_loc_descriptor (rtx rtl, machine_mod
>  	  mem_loc_result->dw_loc_oprnd1.v.val_die_ref.external = 0;
>  	  mem_loc_result->dw_loc_oprnd2.val_class
>  	    = dw_val_class_wide_int;
> -	  mem_loc_result->dw_loc_oprnd2.v.val_wide = ggc_alloc<rwide_int> ();
> -	  *mem_loc_result->dw_loc_oprnd2.v.val_wide = rtx_mode_t (rtl, mode);
> +	  mem_loc_result->dw_loc_oprnd2.v.val_wide
> +	    = alloc_dw_wide_int (rtx_mode_t (rtl, mode));
>  	}
>        break;
>  
> @@ -17288,8 +17298,8 @@ loc_descriptor (rtx rtl, machine_mode mo
>  	  loc_result = new_loc_descr (DW_OP_implicit_value,
>  				      GET_MODE_SIZE (int_mode), 0);
>  	  loc_result->dw_loc_oprnd2.val_class = dw_val_class_wide_int;
> -	  loc_result->dw_loc_oprnd2.v.val_wide = ggc_alloc<rwide_int> ();
> -	  *loc_result->dw_loc_oprnd2.v.val_wide = rtx_mode_t (rtl, int_mode);
> +	  loc_result->dw_loc_oprnd2.v.val_wide
> +	    = alloc_dw_wide_int (rtx_mode_t (rtl, int_mode));
>  	}
>        break;
>  
> @@ -20189,7 +20199,7 @@ extract_int (const unsigned char *src, u
>  /* Writes wide_int values to dw_vec_const array.  */
>  
>  static void
> -insert_wide_int (const rwide_int &val, unsigned char *dest, int elt_size)
> +insert_wide_int (const wide_int_ref &val, unsigned char *dest, int elt_size)
>  {
>    int i;
>  
> @@ -20274,8 +20284,7 @@ add_const_value_attribute (dw_die_ref di
>  	  && (GET_MODE_PRECISION (int_mode)
>  	      & (HOST_BITS_PER_WIDE_INT - 1)) == 0)
>  	{
> -	  rwide_int w = rtx_mode_t (rtl, int_mode);
> -	  add_AT_wide (die, DW_AT_const_value, w);
> +	  add_AT_wide (die, DW_AT_const_value, rtx_mode_t (rtl, int_mode));
>  	  return true;
>  	}
>        return false;
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

  reply	other threads:[~2023-10-10  9:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-09 10:55 [PATCH] wide-int: Allow up to 16320 bits wide_int and change widest_int precision to 32640 bits [PR102989] Jakub Jelinek
2023-10-09 12:54 ` Richard Sandiford
2023-10-09 13:44   ` Jakub Jelinek
2023-10-09 18:28     ` Jakub Jelinek
2023-10-10 17:41       ` Richard Sandiford
2023-10-10 18:13         ` Jakub Jelinek
2023-10-09 14:59 ` [PATCH] wide-int: Remove rwide_int, introduce dw_wide_int Jakub Jelinek
2023-10-10  9:30   ` Richard Biener [this message]
2023-10-10  9:49     ` Jakub Jelinek
2023-10-10 13:42     ` [PATCH] dwarf2out: Stop using wide_int in GC structures Jakub Jelinek
2023-10-10 13:43       ` Richard Biener

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=nycvar.YFH.7.77.849.2310100930140.5561@jbgna.fhfr.qr \
    --to=rguenther@suse.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=richard.sandiford@arm.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).