Message ID | 20230208211419.1583473-1-apinski@marvell.com |
---|---|
State | New |
Headers | show |
Series | When simplifing BFR of an insert, require a mode precision integral type (PR108688) | expand |
On Wed, Feb 8, 2023 at 10:15 PM Andrew Pinski via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > The same problem as PR 88739 has crept in but > this time in match.pd when simplifying bit_field_ref of > an bit_insert. That is we are generating a BIT_FIELD_REF > of a non-mode-precision integral type. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. Hmm. Can we handle the case of the extraction exactly covering the insertion separately then and simplify to plain @1? Thanks, Richard. > PR tree-optimization/108688 > > gcc/ChangeLog: > > * match.pd (bit_field_ref [bit_insert]): > Avoid generating BIT_FIELD_REFs of non-mode-precision > integral operands > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/compile/pr108688-1.c: New test. > --- > gcc/match.pd | 4 +++- > gcc/testsuite/gcc.c-torture/compile/pr108688-1.c | 15 +++++++++++++++ > 2 files changed, 18 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr108688-1.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index c9e8bebede2..d8353072822 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7487,7 +7487,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > isize = tree_to_uhwi (TYPE_SIZE (TREE_TYPE (@1))); > } > (switch > - (if (wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) > + (if ((!INTEGRAL_TYPE_P (TREE_TYPE (@1)) > + || type_has_mode_precision_p (TREE_TYPE (@1))) > + && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) > && wi::leu_p (wi::to_wide (@rpos) + wi::to_wide (@rsize), > wi::to_wide (@ipos) + isize)) > (BIT_FIELD_REF @1 @rsize { wide_int_to_tree (bitsizetype, > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c b/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c > new file mode 100644 > index 00000000000..43d782d62bd > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c > @@ -0,0 +1,15 @@ > + > + > +union U { signed int d : 7; signed int e : 2; } u; > +int a, b; > + > +void > +foo (void) > +{ > + for (int i = 0; i < 64; i++) > + { > + u.d = a; > + u.e ^= b; > + } > +} > + > -- > 2.17.1 >
On Thu, Feb 09, 2023 at 09:16:17AM +0100, Richard Biener via Gcc-patches wrote: > On Wed, Feb 8, 2023 at 10:15 PM Andrew Pinski via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > The same problem as PR 88739 has crept in but > > this time in match.pd when simplifying bit_field_ref of > > an bit_insert. That is we are generating a BIT_FIELD_REF > > of a non-mode-precision integral type. > > > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > Hmm. Can we handle the case of the extraction exactly covering the > insertion separately then and simplify to plain @1? I was suggesting that in the PR. Here it is as an incremental patch on top of Andrew's patch. Ok for trunk if it passes bootstrap/regtest? On the newly added testcase the ifcvt-folding difference without/with the incremental patch is: --- pr108688.c.171t.ifcvt_ 2023-02-09 10:47:30.169916845 +0100 +++ pr108688.c.171t.ifcvt 2023-02-09 10:48:44.942793453 +0100 @@ -25,6 +25,8 @@ Number of blocks in CFG: 11 Number of blocks to update: 5 ( 45%) +Applying pattern match.pd:7487, gimple-match.cc:243200 +Applying pattern match.pd:3987, gimple-match.cc:75423 Matching expression match.pd:1677, gimple-match.cc:209 Applying pattern match.pd:1733, gimple-match.cc:109481 Matching expression match.pd:2393, gimple-match.cc:852 @@ -70,7 +72,6 @@ void foo () signed char _29; <unnamed-signed:7> _30; unsigned int ivtmp_33; - <unnamed-signed:7> _ifc__35; unsigned char _ifc__37; unsigned char _ifc__38; unsigned char _ifc__39; @@ -91,8 +92,7 @@ void foo () _2 = (<unnamed-signed:7>) a.0_1; _ifc__38 = u.D.2741; _ifc__39 = BIT_INSERT_EXPR <_ifc__38, _2, 0 (7 bits)>; - _ifc__35 = BIT_FIELD_REF <_ifc__39, 7, 0>; - _4 = (signed char) _ifc__35; + _4 = (signed char) _2; b.1_5 = b; _6 = (signed char) b.1_5; _7 = _4 ^ _6; 2023-02-09 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/108688 * match.pd (bit_field_ref [bit_insert]): Simplify BIT_FIELD_REF of BIT_INSERT_EXPR extracting exactly all inserted bits even when without mode precision. Formatting fixes. * gcc.c-torture/compile/pr108688-1.c: Add PR number as comment. * gcc.dg/pr108688.c: New test. --- gcc/match.pd.jj 2023-02-09 09:42:53.782418805 +0100 +++ gcc/match.pd 2023-02-09 10:48:06.859365613 +0100 @@ -7475,13 +7475,16 @@ (define_operator_list SYNC_FETCH_AND_AND } (switch (if ((!INTEGRAL_TYPE_P (TREE_TYPE (@1)) - || type_has_mode_precision_p (TREE_TYPE (@1))) - && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) + || type_has_mode_precision_p (TREE_TYPE (@1))) + && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) && wi::leu_p (wi::to_wide (@rpos) + wi::to_wide (@rsize), wi::to_wide (@ipos) + isize)) (BIT_FIELD_REF @1 @rsize { wide_int_to_tree (bitsizetype, - wi::to_wide (@rpos) + wi::to_wide (@rpos) - wi::to_wide (@ipos)); })) + (if (wi::eq_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) + && compare_tree_int (@rsize, isize) == 0) + (convert @1)) (if (wi::geu_p (wi::to_wide (@ipos), wi::to_wide (@rpos) + wi::to_wide (@rsize)) || wi::geu_p (wi::to_wide (@rpos), --- gcc/testsuite/gcc.c-torture/compile/pr108688-1.c.jj 2023-02-09 09:42:53.782418805 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr108688-1.c 2023-02-09 10:34:18.339817796 +0100 @@ -1,4 +1,4 @@ - +/* PR tree-optimization/108688 */ union U { signed int d : 7; signed int e : 2; } u; int a, b; @@ -12,4 +12,3 @@ foo (void) u.e ^= b; } } - --- gcc/testsuite/gcc.dg/pr108688.c.jj 2023-02-09 10:50:27.468253110 +0100 +++ gcc/testsuite/gcc.dg/pr108688.c 2023-02-09 10:50:16.383419646 +0100 @@ -0,0 +1,16 @@ +/* PR tree-optimization/108688 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-pre -fno-tree-fre -fno-tree-dominator-opts -fno-tree-loop-im -fno-code-hoisting" } */ + +union U { signed int d : 7; signed int e : 2; } u; +int a, b; + +void +foo (void) +{ + for (int i = 0; i < 64; i++) + { + u.d = a; + u.d ^= b; + } +} Jakub
On Thu, Feb 9, 2023 at 10:57 AM Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Feb 09, 2023 at 09:16:17AM +0100, Richard Biener via Gcc-patches wrote: > > On Wed, Feb 8, 2023 at 10:15 PM Andrew Pinski via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > The same problem as PR 88739 has crept in but > > > this time in match.pd when simplifying bit_field_ref of > > > an bit_insert. That is we are generating a BIT_FIELD_REF > > > of a non-mode-precision integral type. > > > > > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. > > > > Hmm. Can we handle the case of the extraction exactly covering the > > insertion separately then and simplify to plain @1? > > I was suggesting that in the PR. Here it is as an incremental patch > on top of Andrew's patch. Ok for trunk if it passes bootstrap/regtest? Yes. Thanks, Richard. > On the newly added testcase the ifcvt-folding difference without/with the > incremental patch is: > --- pr108688.c.171t.ifcvt_ 2023-02-09 10:47:30.169916845 +0100 > +++ pr108688.c.171t.ifcvt 2023-02-09 10:48:44.942793453 +0100 > @@ -25,6 +25,8 @@ Number of blocks in CFG: 11 > Number of blocks to update: 5 ( 45%) > > > +Applying pattern match.pd:7487, gimple-match.cc:243200 > +Applying pattern match.pd:3987, gimple-match.cc:75423 > Matching expression match.pd:1677, gimple-match.cc:209 > Applying pattern match.pd:1733, gimple-match.cc:109481 > Matching expression match.pd:2393, gimple-match.cc:852 > @@ -70,7 +72,6 @@ void foo () > signed char _29; > <unnamed-signed:7> _30; > unsigned int ivtmp_33; > - <unnamed-signed:7> _ifc__35; > unsigned char _ifc__37; > unsigned char _ifc__38; > unsigned char _ifc__39; > @@ -91,8 +92,7 @@ void foo () > _2 = (<unnamed-signed:7>) a.0_1; > _ifc__38 = u.D.2741; > _ifc__39 = BIT_INSERT_EXPR <_ifc__38, _2, 0 (7 bits)>; > - _ifc__35 = BIT_FIELD_REF <_ifc__39, 7, 0>; > - _4 = (signed char) _ifc__35; > + _4 = (signed char) _2; > b.1_5 = b; > _6 = (signed char) b.1_5; > _7 = _4 ^ _6; > > 2023-02-09 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/108688 > * match.pd (bit_field_ref [bit_insert]): Simplify BIT_FIELD_REF > of BIT_INSERT_EXPR extracting exactly all inserted bits even > when without mode precision. Formatting fixes. > > * gcc.c-torture/compile/pr108688-1.c: Add PR number as comment. > * gcc.dg/pr108688.c: New test. > > --- gcc/match.pd.jj 2023-02-09 09:42:53.782418805 +0100 > +++ gcc/match.pd 2023-02-09 10:48:06.859365613 +0100 > @@ -7475,13 +7475,16 @@ (define_operator_list SYNC_FETCH_AND_AND > } > (switch > (if ((!INTEGRAL_TYPE_P (TREE_TYPE (@1)) > - || type_has_mode_precision_p (TREE_TYPE (@1))) > - && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) > + || type_has_mode_precision_p (TREE_TYPE (@1))) > + && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) > && wi::leu_p (wi::to_wide (@rpos) + wi::to_wide (@rsize), > wi::to_wide (@ipos) + isize)) > (BIT_FIELD_REF @1 @rsize { wide_int_to_tree (bitsizetype, > - wi::to_wide (@rpos) > + wi::to_wide (@rpos) > - wi::to_wide (@ipos)); })) > + (if (wi::eq_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) > + && compare_tree_int (@rsize, isize) == 0) > + (convert @1)) > (if (wi::geu_p (wi::to_wide (@ipos), > wi::to_wide (@rpos) + wi::to_wide (@rsize)) > || wi::geu_p (wi::to_wide (@rpos), > --- gcc/testsuite/gcc.c-torture/compile/pr108688-1.c.jj 2023-02-09 09:42:53.782418805 +0100 > +++ gcc/testsuite/gcc.c-torture/compile/pr108688-1.c 2023-02-09 10:34:18.339817796 +0100 > @@ -1,4 +1,4 @@ > - > +/* PR tree-optimization/108688 */ > > union U { signed int d : 7; signed int e : 2; } u; > int a, b; > @@ -12,4 +12,3 @@ foo (void) > u.e ^= b; > } > } > - > --- gcc/testsuite/gcc.dg/pr108688.c.jj 2023-02-09 10:50:27.468253110 +0100 > +++ gcc/testsuite/gcc.dg/pr108688.c 2023-02-09 10:50:16.383419646 +0100 > @@ -0,0 +1,16 @@ > +/* PR tree-optimization/108688 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-tree-pre -fno-tree-fre -fno-tree-dominator-opts -fno-tree-loop-im -fno-code-hoisting" } */ > + > +union U { signed int d : 7; signed int e : 2; } u; > +int a, b; > + > +void > +foo (void) > +{ > + for (int i = 0; i < 64; i++) > + { > + u.d = a; > + u.d ^= b; > + } > +} > > > Jakub >
diff --git a/gcc/match.pd b/gcc/match.pd index c9e8bebede2..d8353072822 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7487,7 +7487,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) isize = tree_to_uhwi (TYPE_SIZE (TREE_TYPE (@1))); } (switch - (if (wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) + (if ((!INTEGRAL_TYPE_P (TREE_TYPE (@1)) + || type_has_mode_precision_p (TREE_TYPE (@1))) + && wi::leu_p (wi::to_wide (@ipos), wi::to_wide (@rpos)) && wi::leu_p (wi::to_wide (@rpos) + wi::to_wide (@rsize), wi::to_wide (@ipos) + isize)) (BIT_FIELD_REF @1 @rsize { wide_int_to_tree (bitsizetype, diff --git a/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c b/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c new file mode 100644 index 00000000000..43d782d62bd --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr108688-1.c @@ -0,0 +1,15 @@ + + +union U { signed int d : 7; signed int e : 2; } u; +int a, b; + +void +foo (void) +{ + for (int i = 0; i < 64; i++) + { + u.d = a; + u.e ^= b; + } +} +