diff mbox series

sccvn: Avoid ICEs on _BitInt load BIT_AND_EXPR mask [PR111338]

Message ID ZP9PdYVr65NKA89b@tucnak
State Unresolved
Headers
Series sccvn: Avoid ICEs on _BitInt load BIT_AND_EXPR mask [PR111338] |

Checks

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

Commit Message

Jakub Jelinek Sept. 11, 2023, 5:33 p.m. UTC 
  Hi!

The following testcase ICEs, because vn_walk_cb_data::push_partial_def
uses a fixed size buffer (64 target bytes) for its
construction/deconstruction of partial stores and fails if larger precision
than that is needed, and the PR93582 changes assert push_partial_def
succeeds (and check the various other conditions much earlier when seeing
the BIT_AND_EXPR statement, like CHAR_BIT == 8, BITS_PER_UNIT == 8,
BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN, etc.).  So, just removing the assert
and allowing it fail there doesn't really work and ICEs later on.

The following patch moves the bufsize out of the method and tests it
together with the other checks.

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

BTW, perhaps we could increase the bufsize as well or in addition to
increasing it make the buffer allocated using XALLOCAVEC, but still I think
it is useful to have some upper bound and so I think this patch is useful
even in that case.

2023-09-11  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/111338
	* tree-ssa-sccvn.cc (struct vn_walk_cb_data): Add bufsize non-static
	data member.
	(vn_walk_cb_data::push_partial_def): Remove bufsize variable.
	(visit_nary_op): Avoid the BIT_AND_EXPR with constant rhs2
	optimization if type's precision is too large for
	vn_walk_cb_data::bufsize.

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


	Jakub

Comments

Richard Biener Sept. 12, 2023, 10:19 a.m. UTC | #1 
On Mon, 11 Sep 2023, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because vn_walk_cb_data::push_partial_def
> uses a fixed size buffer (64 target bytes) for its
> construction/deconstruction of partial stores and fails if larger precision
> than that is needed, and the PR93582 changes assert push_partial_def
> succeeds (and check the various other conditions much earlier when seeing
> the BIT_AND_EXPR statement, like CHAR_BIT == 8, BITS_PER_UNIT == 8,
> BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN, etc.).  So, just removing the assert
> and allowing it fail there doesn't really work and ICEs later on.
> 
> The following patch moves the bufsize out of the method and tests it
> together with the other checks.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> BTW, perhaps we could increase the bufsize as well or in addition to
> increasing it make the buffer allocated using XALLOCAVEC, but still I think
> it is useful to have some upper bound and so I think this patch is useful
> even in that case.

Yeah, the size is choosen to match the largest vector mode we currently
have.

Richard.

> 2023-09-11  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/111338
> 	* tree-ssa-sccvn.cc (struct vn_walk_cb_data): Add bufsize non-static
> 	data member.
> 	(vn_walk_cb_data::push_partial_def): Remove bufsize variable.
> 	(visit_nary_op): Avoid the BIT_AND_EXPR with constant rhs2
> 	optimization if type's precision is too large for
> 	vn_walk_cb_data::bufsize.
> 
> 	* gcc.dg/bitint-37.c: New test.
> 
> --- gcc/tree-ssa-sccvn.cc.jj	2023-09-06 17:28:24.232977433 +0200
> +++ gcc/tree-ssa-sccvn.cc	2023-09-08 13:22:27.928158846 +0200
> @@ -1903,6 +1903,7 @@ struct vn_walk_cb_data
>    alias_set_type first_base_set;
>    splay_tree known_ranges;
>    obstack ranges_obstack;
> +  static constexpr HOST_WIDE_INT bufsize = 64;
>  };
>  
>  vn_walk_cb_data::~vn_walk_cb_data ()
> @@ -1973,7 +1974,6 @@ vn_walk_cb_data::push_partial_def (pd_da
>  				   HOST_WIDE_INT offseti,
>  				   HOST_WIDE_INT maxsizei)
>  {
> -  const HOST_WIDE_INT bufsize = 64;
>    /* We're using a fixed buffer for encoding so fail early if the object
>       we want to interpret is bigger.  */
>    if (maxsizei > bufsize * BITS_PER_UNIT
> @@ -5414,6 +5414,7 @@ visit_nary_op (tree lhs, gassign *stmt)
>  	  && CHAR_BIT == 8
>  	  && BITS_PER_UNIT == 8
>  	  && BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN
> +	  && TYPE_PRECISION (type) <= vn_walk_cb_data::bufsize * BITS_PER_UNIT
>  	  && !integer_all_onesp (gimple_assign_rhs2 (stmt))
>  	  && !integer_zerop (gimple_assign_rhs2 (stmt)))
>  	{
> --- gcc/testsuite/gcc.dg/bitint-37.c.jj	2023-09-08 13:27:51.676882523 +0200
> +++ gcc/testsuite/gcc.dg/bitint-37.c	2023-09-08 13:27:22.460268614 +0200
> @@ -0,0 +1,11 @@
> +/* PR middle-end/111338 */
> +/* { dg-do compile { target bitint575 } } */
> +/* { dg-options "-O1" } */
> +
> +_BitInt(575) e;
> +
> +_BitInt(575)
> +foo (void)
> +{
> +  return e & 1;
> +}
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/tree-ssa-sccvn.cc.jj	2023-09-06 17:28:24.232977433 +0200
+++ gcc/tree-ssa-sccvn.cc	2023-09-08 13:22:27.928158846 +0200
@@ -1903,6 +1903,7 @@  struct vn_walk_cb_data
   alias_set_type first_base_set;
   splay_tree known_ranges;
   obstack ranges_obstack;
+  static constexpr HOST_WIDE_INT bufsize = 64;
 };
 
 vn_walk_cb_data::~vn_walk_cb_data ()
@@ -1973,7 +1974,6 @@  vn_walk_cb_data::push_partial_def (pd_da
 				   HOST_WIDE_INT offseti,
 				   HOST_WIDE_INT maxsizei)
 {
-  const HOST_WIDE_INT bufsize = 64;
   /* We're using a fixed buffer for encoding so fail early if the object
      we want to interpret is bigger.  */
   if (maxsizei > bufsize * BITS_PER_UNIT
@@ -5414,6 +5414,7 @@  visit_nary_op (tree lhs, gassign *stmt)
 	  && CHAR_BIT == 8
 	  && BITS_PER_UNIT == 8
 	  && BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN
+	  && TYPE_PRECISION (type) <= vn_walk_cb_data::bufsize * BITS_PER_UNIT
 	  && !integer_all_onesp (gimple_assign_rhs2 (stmt))
 	  && !integer_zerop (gimple_assign_rhs2 (stmt)))
 	{
--- gcc/testsuite/gcc.dg/bitint-37.c.jj	2023-09-08 13:27:51.676882523 +0200
+++ gcc/testsuite/gcc.dg/bitint-37.c	2023-09-08 13:27:22.460268614 +0200
@@ -0,0 +1,11 @@ 
+/* PR middle-end/111338 */
+/* { dg-do compile { target bitint575 } } */
+/* { dg-options "-O1" } */
+
+_BitInt(575) e;
+
+_BitInt(575)
+foo (void)
+{
+  return e & 1;
+}