public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] bitint: Fix handling of conditional bitfield loads [PR114365]
@ 2024-03-20  9:29 Jakub Jelinek
  2024-03-20  9:51 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2024-03-20  9:29 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

Hi!

For the m_var_msb (aka left shift) case of large/huge _BitInt bitfield loads
handle_load adds a PHI node, but I forgot to actually update the temporary
the code later on uses, so the PHI result was unused and the code
incorrectly used something that wasn't valid SSA form.
In particular, we emitted
  if (_29 != 2)
    goto <bb 4>; [80.00%]
  else
    goto <bb 5>; [20.00%]
  
  <bb 4> [local count: 1073741824]:
  _33 = VIEW_CONVERT_EXPR<unsigned long[3]>(s.D.2771)[_31];
  
  <bb 5> [local count: 1073741824]:
  # _34 = PHI <_33(4), 0(3)>
  _35 = _32 >> 31;
  _36 = _33 << 33;
  _37 = _36 | _35;
  _38 = _37 << _19;
where instead of _33 the _36 def stmt should be using _34.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

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

	PR tree-optimization/114365
	* gimple-lower-bitint.cc (bitint_large_huge::handle_load): When adding
	a PHI node, set iv2 to its result afterwards.

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

--- gcc/gimple-lower-bitint.cc.jj	2024-03-16 15:15:22.000000000 +0100
+++ gcc/gimple-lower-bitint.cc	2024-03-19 10:45:31.006649671 +0100
@@ -2026,6 +2026,7 @@ bitint_large_huge::handle_load (gimple *
 	      add_phi_arg (phi, build_zero_cst (m_limb_type),
 			   edge_false, UNKNOWN_LOCATION);
 	      m_gsi = gsi_after_labels (edge_true->dest);
+	      iv2 = iv3;
 	    }
 	}
       g = gimple_build_assign (make_ssa_name (m_limb_type), RSHIFT_EXPR,
--- gcc/testsuite/gcc.dg/bitint-102.c.jj	2024-03-19 10:54:15.317327543 +0100
+++ gcc/testsuite/gcc.dg/bitint-102.c	2024-03-19 10:53:57.162580193 +0100
@@ -0,0 +1,18 @@
+/* PR tree-optimization/114365 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-std=c23 -O2" } */
+
+struct S {
+  int : 31;
+#if __BITINT_MAXWIDTH__ >= 129
+  _BitInt(129) b : 129;
+#else
+  _BitInt(63) b : 63;
+#endif
+} s;
+
+void
+foo (int a)
+{
+  s.b <<= a;
+}

	Jakub


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] bitint: Fix handling of conditional bitfield loads [PR114365]
  2024-03-20  9:29 [PATCH] bitint: Fix handling of conditional bitfield loads [PR114365] Jakub Jelinek
@ 2024-03-20  9:51 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2024-03-20  9:51 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On Wed, 20 Mar 2024, Jakub Jelinek wrote:

> Hi!
> 
> For the m_var_msb (aka left shift) case of large/huge _BitInt bitfield loads
> handle_load adds a PHI node, but I forgot to actually update the temporary
> the code later on uses, so the PHI result was unused and the code
> incorrectly used something that wasn't valid SSA form.
> In particular, we emitted
>   if (_29 != 2)
>     goto <bb 4>; [80.00%]
>   else
>     goto <bb 5>; [20.00%]
>   
>   <bb 4> [local count: 1073741824]:
>   _33 = VIEW_CONVERT_EXPR<unsigned long[3]>(s.D.2771)[_31];
>   
>   <bb 5> [local count: 1073741824]:
>   # _34 = PHI <_33(4), 0(3)>
>   _35 = _32 >> 31;
>   _36 = _33 << 33;
>   _37 = _36 | _35;
>   _38 = _37 << _19;
> where instead of _33 the _36 def stmt should be using _34.
> 
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?

OK.

Richard.

> 2024-03-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/114365
> 	* gimple-lower-bitint.cc (bitint_large_huge::handle_load): When adding
> 	a PHI node, set iv2 to its result afterwards.
> 
> 	* gcc.dg/bitint-102.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj	2024-03-16 15:15:22.000000000 +0100
> +++ gcc/gimple-lower-bitint.cc	2024-03-19 10:45:31.006649671 +0100
> @@ -2026,6 +2026,7 @@ bitint_large_huge::handle_load (gimple *
>  	      add_phi_arg (phi, build_zero_cst (m_limb_type),
>  			   edge_false, UNKNOWN_LOCATION);
>  	      m_gsi = gsi_after_labels (edge_true->dest);
> +	      iv2 = iv3;
>  	    }
>  	}
>        g = gimple_build_assign (make_ssa_name (m_limb_type), RSHIFT_EXPR,
> --- gcc/testsuite/gcc.dg/bitint-102.c.jj	2024-03-19 10:54:15.317327543 +0100
> +++ gcc/testsuite/gcc.dg/bitint-102.c	2024-03-19 10:53:57.162580193 +0100
> @@ -0,0 +1,18 @@
> +/* PR tree-optimization/114365 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23 -O2" } */
> +
> +struct S {
> +  int : 31;
> +#if __BITINT_MAXWIDTH__ >= 129
> +  _BitInt(129) b : 129;
> +#else
> +  _BitInt(63) b : 63;
> +#endif
> +} s;
> +
> +void
> +foo (int a)
> +{
> +  s.b <<= a;
> +}
> 
> 	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)

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-03-20  9:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20  9:29 [PATCH] bitint: Fix handling of conditional bitfield loads [PR114365] Jakub Jelinek
2024-03-20  9:51 ` Richard Biener

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