* [PATCH 0/2] tree_nonzero_bits vs vector and complex types
@ 2022-11-02 21:46 apinski
2022-11-02 21:46 ` [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types apinski
2022-11-02 21:46 ` [PATCH 2/2] Add assert for type on tree_nonzero_bits apinski
0 siblings, 2 replies; 6+ messages in thread
From: apinski @ 2022-11-02 21:46 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
From: Andrew Pinski <apinski@marvell.com>
While looking at older unconfirmed bug reports, I noticed there was
an ubsan found issue and noticed tree_nonzero_bits was being called with
a vector type. How ubsan found it was at the end of tree_nonzero_bits,
did "return wi::shwi (-1, TYPE_PRECISION (TREE_TYPE (t)));" and
it was with a vector of 1 elements which meant precision was 0
as precision stores the log2 of the number of elements in a vector.
Anyways we want to catch these kind of errors of calling tree_nonzero_bits
with a vector or a complex type. And fix the places where it is called.
Thanks,
Andrew Pinski
Andrew Pinski (2):
Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector
types
Add assert for type on tree_nonzero_bits
gcc/fold-const.cc | 3 +++
gcc/match.pd | 25 +++++++++++--------
.../gcc.c-torture/compile/vector-shift-1.c | 8 ++++++
3 files changed, 25 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types 2022-11-02 21:46 [PATCH 0/2] tree_nonzero_bits vs vector and complex types apinski @ 2022-11-02 21:46 ` apinski 2022-11-05 11:44 ` Richard Biener 2022-11-02 21:46 ` [PATCH 2/2] Add assert for type on tree_nonzero_bits apinski 1 sibling, 1 reply; 6+ messages in thread From: apinski @ 2022-11-02 21:46 UTC (permalink / raw) To: gcc-patches; +Cc: Andrew Pinski From: Andrew Pinski <apinski@marvell.com> Even though this PR was reported with an ubsan issue, the problem is tree_nonzero_bits is being called with an expression which is a vector type. This fixes three patterns I noticed which does that. And adds a testcase for one of the patterns. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions gcc/ChangeLog: PR tree-optimization/105532 * match.pd (~(X >> Y) -> ~X >> Y): Check if it is an integral type before calling tree_nonzero_bits. (popcount(X) + popcount(Y)): Likewise. (popcount(X&C1)): Likewise. gcc/testsuite/ChangeLog: * gcc.c-torture/compile/vector-shift-1.c: New test. --- gcc/match.pd | 25 +++++++++++-------- .../gcc.c-torture/compile/vector-shift-1.c | 8 ++++++ 2 files changed, 22 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c diff --git a/gcc/match.pd b/gcc/match.pd index 194ba8f5188..5833e05a926 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -1371,7 +1371,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* For logical right shifts, this is possible only if @0 doesn't have MSB set and the logical right shift is changed into arithmetic shift. */ - (if (!wi::neg_p (tree_nonzero_bits (@0))) + (if (INTEGRAL_TYPE_P (type) + && !wi::neg_p (tree_nonzero_bits (@0))) (with { tree stype = signed_type_for (TREE_TYPE (@0)); } (convert (rshift (bit_not! (convert:stype @0)) @1)))))) @@ -7518,7 +7519,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) /* popcount(X) + popcount(Y) is popcount(X|Y) when X&Y must be zero. */ (simplify (plus (POPCOUNT:s @0) (POPCOUNT:s @1)) - (if (wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) + (if (INTEGRAL_TYPE_P (type) + && wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) (POPCOUNT (bit_ior @0 @1)))) /* popcount(X) == 0 is X == 0, and related (in)equalities. */ @@ -7550,15 +7552,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (for pfun (POPCOUNT PARITY) (simplify (pfun @0) - (with { wide_int nz = tree_nonzero_bits (@0); } - (switch - (if (nz == 1) - (convert @0)) - (if (wi::popcount (nz) == 1) - (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } - (convert (rshift:utype (convert:utype @0) - { build_int_cst (integer_type_node, - wi::ctz (nz)); })))))))) + (if (INTEGRAL_TYPE_P (type)) + (with { wide_int nz = tree_nonzero_bits (@0); } + (switch + (if (nz == 1) + (convert @0)) + (if (wi::popcount (nz) == 1) + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } + (convert (rshift:utype (convert:utype @0) + { build_int_cst (integer_type_node, + wi::ctz (nz)); }))))))))) #if GIMPLE /* 64- and 32-bits branchless implementations of popcount are detected: diff --git a/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c new file mode 100644 index 00000000000..142ea56d5bb --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c @@ -0,0 +1,8 @@ +typedef unsigned char __attribute__((__vector_size__ (1))) U; + +U +foo (U u) +{ + u = u == u; + return (~(u >> 255)); +} -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types 2022-11-02 21:46 ` [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types apinski @ 2022-11-05 11:44 ` Richard Biener 2022-12-20 13:45 ` Richard Biener 0 siblings, 1 reply; 6+ messages in thread From: Richard Biener @ 2022-11-05 11:44 UTC (permalink / raw) To: apinski; +Cc: gcc-patches On Wed, Nov 2, 2022 at 10:47 PM apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Andrew Pinski <apinski@marvell.com> > > Even though this PR was reported with an ubsan issue, the problem is > tree_nonzero_bits is being called with an expression which is a vector type. It seems to me the semantics for vectors should be clear but the users didn't expect that result? > This fixes three patterns I noticed which does that. > And adds a testcase for one of the patterns. > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions OK. > gcc/ChangeLog: > > PR tree-optimization/105532 > * match.pd (~(X >> Y) -> ~X >> Y): Check if it is an integral > type before calling tree_nonzero_bits. > (popcount(X) + popcount(Y)): Likewise. > (popcount(X&C1)): Likewise. > > gcc/testsuite/ChangeLog: > > * gcc.c-torture/compile/vector-shift-1.c: New test. > --- > gcc/match.pd | 25 +++++++++++-------- > .../gcc.c-torture/compile/vector-shift-1.c | 8 ++++++ > 2 files changed, 22 insertions(+), 11 deletions(-) > create mode 100644 gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 194ba8f5188..5833e05a926 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -1371,7 +1371,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* For logical right shifts, this is possible only if @0 doesn't > have MSB set and the logical right shift is changed into > arithmetic shift. */ > - (if (!wi::neg_p (tree_nonzero_bits (@0))) > + (if (INTEGRAL_TYPE_P (type) > + && !wi::neg_p (tree_nonzero_bits (@0))) > (with { tree stype = signed_type_for (TREE_TYPE (@0)); } > (convert (rshift (bit_not! (convert:stype @0)) @1)))))) > > @@ -7518,7 +7519,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > /* popcount(X) + popcount(Y) is popcount(X|Y) when X&Y must be zero. */ > (simplify > (plus (POPCOUNT:s @0) (POPCOUNT:s @1)) > - (if (wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) > + (if (INTEGRAL_TYPE_P (type) > + && wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) > (POPCOUNT (bit_ior @0 @1)))) > > /* popcount(X) == 0 is X == 0, and related (in)equalities. */ > @@ -7550,15 +7552,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > (for pfun (POPCOUNT PARITY) > (simplify > (pfun @0) > - (with { wide_int nz = tree_nonzero_bits (@0); } > - (switch > - (if (nz == 1) > - (convert @0)) > - (if (wi::popcount (nz) == 1) > - (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } > - (convert (rshift:utype (convert:utype @0) > - { build_int_cst (integer_type_node, > - wi::ctz (nz)); })))))))) > + (if (INTEGRAL_TYPE_P (type)) > + (with { wide_int nz = tree_nonzero_bits (@0); } > + (switch > + (if (nz == 1) > + (convert @0)) > + (if (wi::popcount (nz) == 1) > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } > + (convert (rshift:utype (convert:utype @0) > + { build_int_cst (integer_type_node, > + wi::ctz (nz)); }))))))))) > > #if GIMPLE > /* 64- and 32-bits branchless implementations of popcount are detected: > diff --git a/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > new file mode 100644 > index 00000000000..142ea56d5bb > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > @@ -0,0 +1,8 @@ > +typedef unsigned char __attribute__((__vector_size__ (1))) U; > + > +U > +foo (U u) > +{ > + u = u == u; > + return (~(u >> 255)); > +} > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types 2022-11-05 11:44 ` Richard Biener @ 2022-12-20 13:45 ` Richard Biener 0 siblings, 0 replies; 6+ messages in thread From: Richard Biener @ 2022-12-20 13:45 UTC (permalink / raw) To: apinski; +Cc: gcc-patches On Sat, Nov 5, 2022 at 12:44 PM Richard Biener <richard.guenther@gmail.com> wrote: > > On Wed, Nov 2, 2022 at 10:47 PM apinski--- via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > From: Andrew Pinski <apinski@marvell.com> > > > > Even though this PR was reported with an ubsan issue, the problem is > > tree_nonzero_bits is being called with an expression which is a vector type. > > It seems to me the semantics > for vectors should be clear but the users didn't expect that result? > > > This fixes three patterns I noticed which does that. > > And adds a testcase for one of the patterns. > > > > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions > > OK. You didn't push this yet? > > gcc/ChangeLog: > > > > PR tree-optimization/105532 > > * match.pd (~(X >> Y) -> ~X >> Y): Check if it is an integral > > type before calling tree_nonzero_bits. > > (popcount(X) + popcount(Y)): Likewise. > > (popcount(X&C1)): Likewise. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.c-torture/compile/vector-shift-1.c: New test. > > --- > > gcc/match.pd | 25 +++++++++++-------- > > .../gcc.c-torture/compile/vector-shift-1.c | 8 ++++++ > > 2 files changed, 22 insertions(+), 11 deletions(-) > > create mode 100644 gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 194ba8f5188..5833e05a926 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -1371,7 +1371,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > /* For logical right shifts, this is possible only if @0 doesn't > > have MSB set and the logical right shift is changed into > > arithmetic shift. */ > > - (if (!wi::neg_p (tree_nonzero_bits (@0))) > > + (if (INTEGRAL_TYPE_P (type) > > + && !wi::neg_p (tree_nonzero_bits (@0))) > > (with { tree stype = signed_type_for (TREE_TYPE (@0)); } > > (convert (rshift (bit_not! (convert:stype @0)) @1)))))) > > > > @@ -7518,7 +7519,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > /* popcount(X) + popcount(Y) is popcount(X|Y) when X&Y must be zero. */ > > (simplify > > (plus (POPCOUNT:s @0) (POPCOUNT:s @1)) > > - (if (wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) > > + (if (INTEGRAL_TYPE_P (type) > > + && wi::bit_and (tree_nonzero_bits (@0), tree_nonzero_bits (@1)) == 0) > > (POPCOUNT (bit_ior @0 @1)))) > > > > /* popcount(X) == 0 is X == 0, and related (in)equalities. */ > > @@ -7550,15 +7552,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) > > (for pfun (POPCOUNT PARITY) > > (simplify > > (pfun @0) > > - (with { wide_int nz = tree_nonzero_bits (@0); } > > - (switch > > - (if (nz == 1) > > - (convert @0)) > > - (if (wi::popcount (nz) == 1) > > - (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } > > - (convert (rshift:utype (convert:utype @0) > > - { build_int_cst (integer_type_node, > > - wi::ctz (nz)); })))))))) > > + (if (INTEGRAL_TYPE_P (type)) > > + (with { wide_int nz = tree_nonzero_bits (@0); } > > + (switch > > + (if (nz == 1) > > + (convert @0)) > > + (if (wi::popcount (nz) == 1) > > + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } > > + (convert (rshift:utype (convert:utype @0) > > + { build_int_cst (integer_type_node, > > + wi::ctz (nz)); }))))))))) > > > > #if GIMPLE > > /* 64- and 32-bits branchless implementations of popcount are detected: > > diff --git a/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > > new file mode 100644 > > index 00000000000..142ea56d5bb > > --- /dev/null > > +++ b/gcc/testsuite/gcc.c-torture/compile/vector-shift-1.c > > @@ -0,0 +1,8 @@ > > +typedef unsigned char __attribute__((__vector_size__ (1))) U; > > + > > +U > > +foo (U u) > > +{ > > + u = u == u; > > + return (~(u >> 255)); > > +} > > -- > > 2.17.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] Add assert for type on tree_nonzero_bits 2022-11-02 21:46 [PATCH 0/2] tree_nonzero_bits vs vector and complex types apinski 2022-11-02 21:46 ` [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types apinski @ 2022-11-02 21:46 ` apinski 2022-11-05 11:47 ` Richard Biener 1 sibling, 1 reply; 6+ messages in thread From: apinski @ 2022-11-02 21:46 UTC (permalink / raw) To: gcc-patches; +Cc: Andrew Pinski From: Andrew Pinski <apinski@marvell.com> Right now anyone could call tree_nonzero_bits with either complex or vector types and this will return the wrong thing. So just assert that nobody calls it with this. OK? Bootstrapped and tested with no regressions on x86_64-linux-gnu. gcc/ChangeLog: * fold-const.cc (tree_nonzero_bits): Add assert. --- gcc/fold-const.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index 7e1ea58518b..3ccac9b28df 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -16567,6 +16567,9 @@ c_getstr (tree str) wide_int tree_nonzero_bits (const_tree t) { + gcc_assert (TREE_CODE (TREE_TYPE (t)) != VECTOR_TYPE + && TREE_CODE (TREE_TYPE (t)) != COMPLEX_TYPE); + switch (TREE_CODE (t)) { case INTEGER_CST: -- 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] Add assert for type on tree_nonzero_bits 2022-11-02 21:46 ` [PATCH 2/2] Add assert for type on tree_nonzero_bits apinski @ 2022-11-05 11:47 ` Richard Biener 0 siblings, 0 replies; 6+ messages in thread From: Richard Biener @ 2022-11-05 11:47 UTC (permalink / raw) To: apinski; +Cc: gcc-patches On Wed, Nov 2, 2022 at 10:48 PM apinski--- via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > From: Andrew Pinski <apinski@marvell.com> > > Right now anyone could call tree_nonzero_bits with > either complex or vector types and this will return > the wrong thing. So just assert that nobody calls > it with this. > > OK? Bootstrapped and tested with no regressions on x86_64-linux-gnu. The function comment needs adjustment. To me, nonzero bits of a v4si would simply include all bits of v4si - isn't that what happens? I see the SSA get_nonzero_bits uses element_precision but tree_nonzero_bits uses TYPE_PRECISON as fallback, so I guess the assert is correct. Still the function comment should be amended, also ... > gcc/ChangeLog: > > * fold-const.cc (tree_nonzero_bits): Add > assert. > --- > gcc/fold-const.cc | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > index 7e1ea58518b..3ccac9b28df 100644 > --- a/gcc/fold-const.cc > +++ b/gcc/fold-const.cc > @@ -16567,6 +16567,9 @@ c_getstr (tree str) > wide_int > tree_nonzero_bits (const_tree t) > { > + gcc_assert (TREE_CODE (TREE_TYPE (t)) != VECTOR_TYPE > + && TREE_CODE (TREE_TYPE (t)) != COMPLEX_TYPE); ... please perform a "positive" test. Like INTEGRAL_TYPE_P (TREE_TYPE (t)) || POINTER_TYPE_P (TREE_TYPE (t)) Richard. > + > switch (TREE_CODE (t)) > { > case INTEGER_CST: > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-20 13:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-02 21:46 [PATCH 0/2] tree_nonzero_bits vs vector and complex types apinski 2022-11-02 21:46 ` [PATCH 1/2] Fix PR 105532: match.pd patterns calling tree_nonzero_bits with vector types apinski 2022-11-05 11:44 ` Richard Biener 2022-12-20 13:45 ` Richard Biener 2022-11-02 21:46 ` [PATCH 2/2] Add assert for type on tree_nonzero_bits apinski 2022-11-05 11:47 ` 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).