fold-const: Fold larger VIEW_CONVERT_EXPRs [PR113462]

Message ID Za5SnK4O+RTJClzB@tucnak
State Unresolved
Headers
Series fold-const: Fold larger VIEW_CONVERT_EXPRs [PR113462] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Jan. 22, 2024, 11:33 a.m. UTC
  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  <jakub@redhat.com>

	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.



	Jakub
  

Comments

Richard Biener Jan. 22, 2024, 11:53 a.m. UTC | #1
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  <jakub@redhat.com>
> 
> 	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
> 
>
  

Patch

--- 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