[PATCH] bitint: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[PATCH] bitint: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Jakub Jelinek]

Hi!

The following testcase ICEs, because update-address-taken subpass of
fre5 rewrites
  _BitInt(128) b;
  vector(16) unsigned char _3;

  <bb 2> [local count: 1073741824]:
  _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
  MEM <vector(16) unsigned char> [(char * {ref-all})&b] = _3;
  b ={v} {CLOBBER(eos)};
to
  _BitInt(128) b;
  vector(16) unsigned char _3;

  <bb 2> [local count: 1073741824]:
  _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
  b_5 = VIEW_CONVERT_EXPR<_BitInt(128)>(_3);
but we can't have large/huge _BitInt vars in SSA form after the bitint
lowering except for function arguments loaded from memory, as expansion
isn't able to deal with those, it relies on bitint lowering to lower
those operations.
The following patch fixes that by not clearing TREE_ADDRESSABLE for
large/huge _BitInt vars after bitint lowering, such that we don't
rewrite them into SSA form.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/114278
	* tree-ssa.cc (maybe_optimize_var): Punt on large/huge _BitInt
	vars after bitint lowering.

	* gcc.dg/bitint-99.c: New test.

--- gcc/tree-ssa.cc.jj	2024-01-03 11:51:39.902615009 +0100
+++ gcc/tree-ssa.cc	2024-03-08 14:24:11.844821915 +0100
@@ -1753,7 +1753,11 @@ maybe_optimize_var (tree var, bitmap add
   /* Global Variables, result decls cannot be changed.  */
   if (is_global_var (var)
       || TREE_CODE (var) == RESULT_DECL
-      || bitmap_bit_p (addresses_taken, DECL_UID (var)))
+      || bitmap_bit_p (addresses_taken, DECL_UID (var))
+      || (TREE_CODE (TREE_TYPE (var)) == BITINT_TYPE
+	  /* Don't change large/huge _BitInt vars after _BitInt lowering.  */
+	  && (cfun->curr_properties & PROP_gimple_lbitint) != 0
+	  && TYPE_PRECISION (TREE_TYPE (var)) > MAX_FIXED_MODE_SIZE))
     return;
 
   bool maybe_reg = false;
--- gcc/testsuite/gcc.dg/bitint-99.c.jj	2024-03-08 14:26:17.658069942 +0100
+++ gcc/testsuite/gcc.dg/bitint-99.c	2024-03-08 14:25:36.292645965 +0100
@@ -0,0 +1,26 @@
+/* PR tree-optimization/114278 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O2 -fno-tree-dce -fno-tree-dse -fno-tree-ccp" } */
+/* { dg-additional-options "-mavx2" { target i?86-*-* x86_64-*-* } } */
+
+void
+foo (void *p)
+{
+  _BitInt(64) b = *(_BitInt(64) *) __builtin_memmove (&b, p, sizeof (_BitInt(64)));
+}
+
+#if __BITINT_MAXWIDTH__ >= 128
+void
+bar (void *p)
+{
+  _BitInt(128) b = *(_BitInt(128) *) __builtin_memmove (&b, p, sizeof (_BitInt(128)));
+}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 256
+void
+baz (void *p)
+{
+  _BitInt(256) b = *(_BitInt(256) *) __builtin_memmove (&b, p, sizeof (_BitInt(256)));
+}
+#endif

	Jakub

Re: [PATCH] bitint: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Richard Biener]

> Am 09.03.2024 um 09:28 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> The following testcase ICEs, because update-address-taken subpass of
> fre5 rewrites
>  _BitInt(128) b;
>  vector(16) unsigned char _3;
> 
>  <bb 2> [local count: 1073741824]:
>  _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
>  MEM <vector(16) unsigned char> [(char * {ref-all})&b] = _3;
>  b ={v} {CLOBBER(eos)};
> to
>  _BitInt(128) b;
>  vector(16) unsigned char _3;
> 
>  <bb 2> [local count: 1073741824]:
>  _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
>  b_5 = VIEW_CONVERT_EXPR<_BitInt(128)>(_3);
> but we can't have large/huge _BitInt vars in SSA form after the bitint
> lowering except for function arguments loaded from memory, as expansion
> isn't able to deal with those, it relies on bitint lowering to lower
> those operations.
> The following patch fixes that by not clearing TREE_ADDRESSABLE for
> large/huge _BitInt vars after bitint lowering, such that we don't
> rewrite them into SSA form.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 

Ideally we’d clear TREE_ADDRESSABLE but set DECL_NOT_GIMPLE_REG, I think the analysis where we check the base would be a more appropriate place to enforce that.

Richard 

> 2024-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR tree-optimization/114278
>    * tree-ssa.cc (maybe_optimize_var): Punt on large/huge _BitInt
>    vars after bitint lowering.
> 
>    * gcc.dg/bitint-99.c: New test.
> 
> --- gcc/tree-ssa.cc.jj    2024-01-03 11:51:39.902615009 +0100
> +++ gcc/tree-ssa.cc    2024-03-08 14:24:11.844821915 +0100
> @@ -1753,7 +1753,11 @@ maybe_optimize_var (tree var, bitmap add
>   /* Global Variables, result decls cannot be changed.  */
>   if (is_global_var (var)
>       || TREE_CODE (var) == RESULT_DECL
> -      || bitmap_bit_p (addresses_taken, DECL_UID (var)))
> +      || bitmap_bit_p (addresses_taken, DECL_UID (var))
> +      || (TREE_CODE (TREE_TYPE (var)) == BITINT_TYPE
> +      /* Don't change large/huge _BitInt vars after _BitInt lowering.  */
> +      && (cfun->curr_properties & PROP_gimple_lbitint) != 0
> +      && TYPE_PRECISION (TREE_TYPE (var)) > MAX_FIXED_MODE_SIZE))
>     return;
> 
>   bool maybe_reg = false;
> --- gcc/testsuite/gcc.dg/bitint-99.c.jj    2024-03-08 14:26:17.658069942 +0100
> +++ gcc/testsuite/gcc.dg/bitint-99.c    2024-03-08 14:25:36.292645965 +0100
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/114278 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-O2 -fno-tree-dce -fno-tree-dse -fno-tree-ccp" } */
> +/* { dg-additional-options "-mavx2" { target i?86-*-* x86_64-*-* } } */
> +
> +void
> +foo (void *p)
> +{
> +  _BitInt(64) b = *(_BitInt(64) *) __builtin_memmove (&b, p, sizeof (_BitInt(64)));
> +}
> +
> +#if __BITINT_MAXWIDTH__ >= 128
> +void
> +bar (void *p)
> +{
> +  _BitInt(128) b = *(_BitInt(128) *) __builtin_memmove (&b, p, sizeof (_BitInt(128)));
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 256
> +void
> +baz (void *p)
> +{
> +  _BitInt(256) b = *(_BitInt(256) *) __builtin_memmove (&b, p, sizeof (_BitInt(256)));
> +}
> +#endif
> 
>    Jakub
> 

[PATCH] bitint, v2: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Jakub Jelinek]

On Sat, Mar 09, 2024 at 12:25:42PM +0100, Richard Biener wrote:
> Ideally we’d clear TREE_ADDRESSABLE but set DECL_NOT_GIMPLE_REG,
> I think the analysis where we check the base would be a more
> appropriate place to enforce that.

So like this?

Bootstrapped/regtested on x86_64-linux and i686-linux.

2024-03-11  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/114278
	* tree-ssa.cc (maybe_optimize_var): If large/huge _BitInt vars are no
	longer addressable, set DECL_NOT_GIMPLE_REG_P on them.

	* gcc.dg/bitint-99.c: New test.

--- gcc/tree-ssa.cc.jj	2024-01-03 11:51:39.902615009 +0100
+++ gcc/tree-ssa.cc	2024-03-09 23:34:12.469223987 +0100
@@ -1785,6 +1785,20 @@ maybe_optimize_var (tree var, bitmap add
 	      fprintf (dump_file, "\n");
 	    }
 	}
+      else if (TREE_CODE (TREE_TYPE (var)) == BITINT_TYPE
+	       && (cfun->curr_properties & PROP_gimple_lbitint) != 0
+	       && TYPE_PRECISION (TREE_TYPE (var)) > MAX_FIXED_MODE_SIZE)
+	{
+	  /* Don't rewrite large/huge _BitInt vars after _BitInt lowering
+	     into SSA form.  */
+	  DECL_NOT_GIMPLE_REG_P (var) = 1;
+	  if (dump_file)
+	    {
+	      fprintf (dump_file, "_BitInt var after its lowering: ");
+	      print_generic_expr (dump_file, var);
+	      fprintf (dump_file, "\n");
+	    }
+	}
       else if (DECL_NOT_GIMPLE_REG_P (var))
 	{
 	  maybe_reg = true;
--- gcc/testsuite/gcc.dg/bitint-99.c.jj	2024-03-08 14:26:17.658069942 +0100
+++ gcc/testsuite/gcc.dg/bitint-99.c	2024-03-08 14:25:36.292645965 +0100
@@ -0,0 +1,26 @@
+/* PR tree-optimization/114278 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O2 -fno-tree-dce -fno-tree-dse -fno-tree-ccp" } */
+/* { dg-additional-options "-mavx2" { target i?86-*-* x86_64-*-* } } */
+
+void
+foo (void *p)
+{
+  _BitInt(64) b = *(_BitInt(64) *) __builtin_memmove (&b, p, sizeof (_BitInt(64)));
+}
+
+#if __BITINT_MAXWIDTH__ >= 128
+void
+bar (void *p)
+{
+  _BitInt(128) b = *(_BitInt(128) *) __builtin_memmove (&b, p, sizeof (_BitInt(128)));
+}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 256
+void
+baz (void *p)
+{
+  _BitInt(256) b = *(_BitInt(256) *) __builtin_memmove (&b, p, sizeof (_BitInt(256)));
+}
+#endif

	Jakub

Re: [PATCH] bitint: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Richard Biener]

On Sat, 9 Mar 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because update-address-taken subpass of
> fre5 rewrites
>   _BitInt(128) b;
>   vector(16) unsigned char _3;
> 
>   <bb 2> [local count: 1073741824]:
>   _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
>   MEM <vector(16) unsigned char> [(char * {ref-all})&b] = _3;
>   b ={v} {CLOBBER(eos)};
> to
>   _BitInt(128) b;
>   vector(16) unsigned char _3;
> 
>   <bb 2> [local count: 1073741824]:
>   _3 = MEM <vector(16) unsigned char> [(char * {ref-all})p_2(D)];
>   b_5 = VIEW_CONVERT_EXPR<_BitInt(128)>(_3);
> but we can't have large/huge _BitInt vars in SSA form after the bitint
> lowering except for function arguments loaded from memory, as expansion
> isn't able to deal with those, it relies on bitint lowering to lower
> those operations.
> The following patch fixes that by not clearing TREE_ADDRESSABLE for
> large/huge _BitInt vars after bitint lowering, such that we don't
> rewrite them into SSA form.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2024-03-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/114278
> 	* tree-ssa.cc (maybe_optimize_var): Punt on large/huge _BitInt
> 	vars after bitint lowering.
> 
> 	* gcc.dg/bitint-99.c: New test.
> 
> --- gcc/tree-ssa.cc.jj	2024-01-03 11:51:39.902615009 +0100
> +++ gcc/tree-ssa.cc	2024-03-08 14:24:11.844821915 +0100
> @@ -1753,7 +1753,11 @@ maybe_optimize_var (tree var, bitmap add
>    /* Global Variables, result decls cannot be changed.  */
>    if (is_global_var (var)
>        || TREE_CODE (var) == RESULT_DECL
> -      || bitmap_bit_p (addresses_taken, DECL_UID (var)))
> +      || bitmap_bit_p (addresses_taken, DECL_UID (var))
> +      || (TREE_CODE (TREE_TYPE (var)) == BITINT_TYPE
> +	  /* Don't change large/huge _BitInt vars after _BitInt lowering.  */
> +	  && (cfun->curr_properties & PROP_gimple_lbitint) != 0
> +	  && TYPE_PRECISION (TREE_TYPE (var)) > MAX_FIXED_MODE_SIZE))
>      return;
>  
>    bool maybe_reg = false;
> --- gcc/testsuite/gcc.dg/bitint-99.c.jj	2024-03-08 14:26:17.658069942 +0100
> +++ gcc/testsuite/gcc.dg/bitint-99.c	2024-03-08 14:25:36.292645965 +0100
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/114278 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-O2 -fno-tree-dce -fno-tree-dse -fno-tree-ccp" } */
> +/* { dg-additional-options "-mavx2" { target i?86-*-* x86_64-*-* } } */
> +
> +void
> +foo (void *p)
> +{
> +  _BitInt(64) b = *(_BitInt(64) *) __builtin_memmove (&b, p, sizeof (_BitInt(64)));
> +}
> +
> +#if __BITINT_MAXWIDTH__ >= 128
> +void
> +bar (void *p)
> +{
> +  _BitInt(128) b = *(_BitInt(128) *) __builtin_memmove (&b, p, sizeof (_BitInt(128)));
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 256
> +void
> +baz (void *p)
> +{
> +  _BitInt(256) b = *(_BitInt(256) *) __builtin_memmove (&b, p, sizeof (_BitInt(256)));
> +}
> +#endif
> 
> 	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)

Re: [PATCH] bitint, v2: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Richard Biener]

On Mon, 11 Mar 2024, Jakub Jelinek wrote:

> On Sat, Mar 09, 2024 at 12:25:42PM +0100, Richard Biener wrote:
> > Ideally we?d clear TREE_ADDRESSABLE but set DECL_NOT_GIMPLE_REG,
> > I think the analysis where we check the base would be a more
> > appropriate place to enforce that.
> 
> So like this?

Hm, I was thinking of non_rewritable_lvalue_p/non_rewritable_mem_ref_base
though that requires duplicating, so I guess handling in maybe_optimize_var 
would work.

I do now wonder whether setting DECL_NOT_GIMPLE_REG_P in bitfield
lowering would prevail?

(sorry for approving the earlier patch now, I was too quick and didn't
remember the discussion)

Richard.

> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2024-03-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/114278
> 	* tree-ssa.cc (maybe_optimize_var): If large/huge _BitInt vars are no
> 	longer addressable, set DECL_NOT_GIMPLE_REG_P on them.
> 
> 	* gcc.dg/bitint-99.c: New test.
> 
> --- gcc/tree-ssa.cc.jj	2024-01-03 11:51:39.902615009 +0100
> +++ gcc/tree-ssa.cc	2024-03-09 23:34:12.469223987 +0100
> @@ -1785,6 +1785,20 @@ maybe_optimize_var (tree var, bitmap add
>  	      fprintf (dump_file, "\n");
>  	    }
>  	}
> +      else if (TREE_CODE (TREE_TYPE (var)) == BITINT_TYPE
> +	       && (cfun->curr_properties & PROP_gimple_lbitint) != 0
> +	       && TYPE_PRECISION (TREE_TYPE (var)) > MAX_FIXED_MODE_SIZE)
> +	{
> +	  /* Don't rewrite large/huge _BitInt vars after _BitInt lowering
> +	     into SSA form.  */
> +	  DECL_NOT_GIMPLE_REG_P (var) = 1;
> +	  if (dump_file)
> +	    {
> +	      fprintf (dump_file, "_BitInt var after its lowering: ");
> +	      print_generic_expr (dump_file, var);
> +	      fprintf (dump_file, "\n");
> +	    }
> +	}
>        else if (DECL_NOT_GIMPLE_REG_P (var))
>  	{
>  	  maybe_reg = true;
> --- gcc/testsuite/gcc.dg/bitint-99.c.jj	2024-03-08 14:26:17.658069942 +0100
> +++ gcc/testsuite/gcc.dg/bitint-99.c	2024-03-08 14:25:36.292645965 +0100
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/114278 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-O2 -fno-tree-dce -fno-tree-dse -fno-tree-ccp" } */
> +/* { dg-additional-options "-mavx2" { target i?86-*-* x86_64-*-* } } */
> +
> +void
> +foo (void *p)
> +{
> +  _BitInt(64) b = *(_BitInt(64) *) __builtin_memmove (&b, p, sizeof (_BitInt(64)));
> +}
> +
> +#if __BITINT_MAXWIDTH__ >= 128
> +void
> +bar (void *p)
> +{
> +  _BitInt(128) b = *(_BitInt(128) *) __builtin_memmove (&b, p, sizeof (_BitInt(128)));
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 256
> +void
> +baz (void *p)
> +{
> +  _BitInt(256) b = *(_BitInt(256) *) __builtin_memmove (&b, p, sizeof (_BitInt(256)));
> +}
> +#endif
> 
> 	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)

Re: [PATCH] bitint, v2: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Jakub Jelinek]

On Mon, Mar 11, 2024 at 11:31:51AM +0100, Richard Biener wrote:
> On Mon, 11 Mar 2024, Jakub Jelinek wrote:
> 
> > On Sat, Mar 09, 2024 at 12:25:42PM +0100, Richard Biener wrote:
> > > Ideally we?d clear TREE_ADDRESSABLE but set DECL_NOT_GIMPLE_REG,
> > > I think the analysis where we check the base would be a more
> > > appropriate place to enforce that.
> > 
> > So like this?
> 
> Hm, I was thinking of non_rewritable_lvalue_p/non_rewritable_mem_ref_base
> though that requires duplicating, so I guess handling in maybe_optimize_var 
> would work.

I was considering it, but it looked like a waste to me, using bitmap bits
for something that is always the case, we don't want to rewrite any
large/huge _BitInt to SSA form after the lowering, not just some of them.

> I do now wonder whether setting DECL_NOT_GIMPLE_REG_P in bitfield
> lowering would prevail?

Guess I can certainly try to set DECL_NOT_GIMPLE_REG_P on the large/huge
_BitInt PARM_DECLs/RESULT_DECLs during bitint lowering even when they are
TREE_ADDRESSABLE at that point; the VAR_DECLs have array types of limbs and
so shouldn't be a problem.

> (sorry for approving the earlier patch now, I was too quick and didn't
> remember the discussion)

Sorry, already committed, I can revert or incrementally adjust.

	Jakub

Re: [PATCH] bitint, v2: Avoid rewriting large/huge _BitInt vars into SSA after bitint lowering [PR114278]

[Richard Biener]

On Mon, 11 Mar 2024, Jakub Jelinek wrote:

> On Mon, Mar 11, 2024 at 11:31:51AM +0100, Richard Biener wrote:
> > On Mon, 11 Mar 2024, Jakub Jelinek wrote:
> > 
> > > On Sat, Mar 09, 2024 at 12:25:42PM +0100, Richard Biener wrote:
> > > > Ideally we?d clear TREE_ADDRESSABLE but set DECL_NOT_GIMPLE_REG,
> > > > I think the analysis where we check the base would be a more
> > > > appropriate place to enforce that.
> > > 
> > > So like this?
> > 
> > Hm, I was thinking of non_rewritable_lvalue_p/non_rewritable_mem_ref_base
> > though that requires duplicating, so I guess handling in maybe_optimize_var 
> > would work.
> 
> I was considering it, but it looked like a waste to me, using bitmap bits
> for something that is always the case, we don't want to rewrite any
> large/huge _BitInt to SSA form after the lowering, not just some of them.
> 
> > I do now wonder whether setting DECL_NOT_GIMPLE_REG_P in bitfield
> > lowering would prevail?
> 
> Guess I can certainly try to set DECL_NOT_GIMPLE_REG_P on the large/huge
> _BitInt PARM_DECLs/RESULT_DECLs during bitint lowering even when they are
> TREE_ADDRESSABLE at that point; the VAR_DECLs have array types of limbs and
> so shouldn't be a problem.

Hmm, looking I think we're going to clear DECL_NOT_GIMPLE_REG_P since
we still have is_gimple_reg_type.

> > (sorry for approving the earlier patch now, I was too quick and didn't
> > remember the discussion)
> 
> Sorry, already committed, I can revert or incrementally adjust.

No problem, I think both patches are OK, the 2nd maybe a bit better
for alias analysis.

Richard.