| Message ID | ZfqshQUSBw/wEl1C@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | bitint: Fix handling of conditional bitfield loads [PR114365] | expand |
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 > >
--- 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; +}
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. Jakub