Re: [PATCH] fold-const: Fold larger VIEW_CONVERT_EXPRs [PR113462]

2024-01-22 Thread Richard Biener
On Mon, 22 Jan 2024, Jakub Jelinek wrote:

> Hi!
> 
> On Mon, Jan 22, 2024 at 11:56:11AM +0100, Richard Biener wrote:
> > > I guess the || total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT
> > > conditions make no sense, all we care is whether it fits in the buffer
> > > or not.
> > > But then there is
> > > fold_view_convert_expr
> > > (and other spots) which use
> > >   /* We support up to 1024-bit values (for GCN/RISC-V V128QImode).  */
> > >   unsigned char buffer[128];
> > > or something similar.
> > > Perhaps we could use XALLOCAVEC there instead (or use it only for the
> > > larger sizes and keep the static buffer for the common case).
> > 
> > Well, yes.  V_C_E folding could do this but then the native_encode_expr
> > API could also allow lazy allocation or so.
> 
> native_encode_expr can't reallocate, it has to fill in whatever buffer it
> has been called with, it can be in the middle of something else etc.
> 
> The following patch is what I meant, I think having some upper bound is
> desirable so that we don't spend too much time trying to VCE fold 2GB
> structures (after all, the APIs also use int for lengths) and similar and 
> passed
> make check-gcc check-g++ -j32 -k GCC_TEST_RUN_EXPENSIVE=1 
> RUNTESTFLAGS="GCC_TEST_RUN_EXPENSIVE=1 dg.exp='*bitint* pr112673.c 
> builtin-stdc-bit-*.c pr112566-2.c pr112511.c' dg-torture.exp=*bitint* 
> dfp.exp=*bitint*"
> (my usual quick test for bitint related changes).  Ok for trunk if it passes
> full bootstrap/regtest?

OK.

Thanks,
Richard.

> 2024-01-22  Jakub Jelinek  
> 
>   PR tree-optimization/113462
>   * fold-const.cc (native_interpret_int): Don't punt if total_bytes
>   is larger than HOST_BITS_PER_DOUBLE_INT / BITS_PER_UNIT.
>   (fold_view_convert_expr): Use XALLOCAVEC buffers for types with
>   sizes between 129 and 8192 bytes.
> 
> --- gcc/fold-const.cc.jj  2024-01-12 10:07:58.202851544 +0100
> +++ gcc/fold-const.cc 2024-01-22 12:09:05.116253393 +0100
> @@ -8773,8 +8773,7 @@ native_interpret_int (tree type, const u
>else
>  total_bytes = GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (type));
>  
> -  if (total_bytes > len
> -  || total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT)
> +  if (total_bytes > len)
>  return NULL_TREE;
>  
>wide_int result = wi::from_buffer (ptr, total_bytes);
> @@ -9329,9 +9328,10 @@ fold_view_convert_vector_encoding (tree
>  static tree
>  fold_view_convert_expr (tree type, tree expr)
>  {
> -  /* We support up to 1024-bit values (for GCN/RISC-V V128QImode).  */
>unsigned char buffer[128];
> +  unsigned char *buf;
>int len;
> +  HOST_WIDE_INT l;
>  
>/* Check that the host and target are sane.  */
>if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
> @@ -9341,11 +9341,23 @@ fold_view_convert_expr (tree type, tree
>  if (tree res = fold_view_convert_vector_encoding (type, expr))
>return res;
>  
> -  len = native_encode_expr (expr, buffer, sizeof (buffer));
> +  l = int_size_in_bytes (type);
> +  if (l > (int) sizeof (buffer)
> +  && l <= WIDE_INT_MAX_PRECISION / BITS_PER_UNIT)
> +{
> +  buf = XALLOCAVEC (unsigned char, l);
> +  len = l;
> +}
> +  else
> +{
> +  buf = buffer;
> +  len = sizeof (buffer);
> +}
> +  len = native_encode_expr (expr, buf, len);
>if (len == 0)
>  return NULL_TREE;
>  
> -  return native_interpret_expr (type, buffer, len);
> +  return native_interpret_expr (type, buf, len);
>  }
>  
>  /* Build an expression for the address of T.  Folds away INDIRECT_REF
> 
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)


[PATCH] fold-const: Fold larger VIEW_CONVERT_EXPRs [PR113462]

2024-01-22 Thread Jakub Jelinek
Hi!

On Mon, Jan 22, 2024 at 11:56:11AM +0100, Richard Biener wrote:
> > I guess the || total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT
> > conditions make no sense, all we care is whether it fits in the buffer
> > or not.
> > But then there is
> > fold_view_convert_expr
> > (and other spots) which use
> >   /* We support up to 1024-bit values (for GCN/RISC-V V128QImode).  */
> >   unsigned char buffer[128];
> > or something similar.
> > Perhaps we could use XALLOCAVEC there instead (or use it only for the
> > larger sizes and keep the static buffer for the common case).
> 
> Well, yes.  V_C_E folding could do this but then the native_encode_expr
> API could also allow lazy allocation or so.

native_encode_expr can't reallocate, it has to fill in whatever buffer it
has been called with, it can be in the middle of something else etc.

The following patch is what I meant, I think having some upper bound is
desirable so that we don't spend too much time trying to VCE fold 2GB
structures (after all, the APIs also use int for lengths) and similar and passed
make check-gcc check-g++ -j32 -k GCC_TEST_RUN_EXPENSIVE=1 
RUNTESTFLAGS="GCC_TEST_RUN_EXPENSIVE=1 dg.exp='*bitint* pr112673.c 
builtin-stdc-bit-*.c pr112566-2.c pr112511.c' dg-torture.exp=*bitint* 
dfp.exp=*bitint*"
(my usual quick test for bitint related changes).  Ok for trunk if it passes
full bootstrap/regtest?

2024-01-22  Jakub Jelinek  

PR tree-optimization/113462
* fold-const.cc (native_interpret_int): Don't punt if total_bytes
is larger than HOST_BITS_PER_DOUBLE_INT / BITS_PER_UNIT.
(fold_view_convert_expr): Use XALLOCAVEC buffers for types with
sizes between 129 and 8192 bytes.

--- gcc/fold-const.cc.jj2024-01-12 10:07:58.202851544 +0100
+++ gcc/fold-const.cc   2024-01-22 12:09:05.116253393 +0100
@@ -8773,8 +8773,7 @@ native_interpret_int (tree type, const u
   else
 total_bytes = GET_MODE_SIZE (SCALAR_INT_TYPE_MODE (type));
 
-  if (total_bytes > len
-  || total_bytes * BITS_PER_UNIT > HOST_BITS_PER_DOUBLE_INT)
+  if (total_bytes > len)
 return NULL_TREE;
 
   wide_int result = wi::from_buffer (ptr, total_bytes);
@@ -9329,9 +9328,10 @@ fold_view_convert_vector_encoding (tree
 static tree
 fold_view_convert_expr (tree type, tree expr)
 {
-  /* We support up to 1024-bit values (for GCN/RISC-V V128QImode).  */
   unsigned char buffer[128];
+  unsigned char *buf;
   int len;
+  HOST_WIDE_INT l;
 
   /* Check that the host and target are sane.  */
   if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
@@ -9341,11 +9341,23 @@ fold_view_convert_expr (tree type, tree
 if (tree res = fold_view_convert_vector_encoding (type, expr))
   return res;
 
-  len = native_encode_expr (expr, buffer, sizeof (buffer));
+  l = int_size_in_bytes (type);
+  if (l > (int) sizeof (buffer)
+  && l <= WIDE_INT_MAX_PRECISION / BITS_PER_UNIT)
+{
+  buf = XALLOCAVEC (unsigned char, l);
+  len = l;
+}
+  else
+{
+  buf = buffer;
+  len = sizeof (buffer);
+}
+  len = native_encode_expr (expr, buf, len);
   if (len == 0)
 return NULL_TREE;
 
-  return native_interpret_expr (type, buffer, len);
+  return native_interpret_expr (type, buf, len);
 }
 
 /* Build an expression for the address of T.  Folds away INDIRECT_REF


Jakub