Message ID | 20220216090309.82939-1-hongtao.liu@intel.com |
---|---|
State | New |
Headers | show |
Series | Restrict the two sources of vect_recog_cond_expr_convert_pattern to be of the same type when convert is extension. | expand |
On Wed, Feb 16, 2022 at 05:03:09PM +0800, liuhongt via Gcc-patches wrote: > > > +(match (cond_expr_convert_p @0 @2 @3 @6) > > > + (cond (simple_comparison@6 @0 @1) (convert@4 @2) (convert@5 @3)) > > > + (if (types_match (TREE_TYPE (@2), TREE_TYPE (@3)) > > > > But in principle @2 or @3 could safely differ in sign, you'd then need to ensure > > to insert sign conversions to @2/@3 to the signedness of @4/@5. > > > It turns out differ in sign is not suitable for extension(but ok for truncation), > because it's zero_extend vs sign_extend. > > The patch add types_match check when convert is extension. > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > And native Bootstrapped and regtested on CLX. > > Ok for trunk? > > gcc/ChangeLog: > > PR tree-optimization/104551 > PR tree-optimization/103771 > * match.pd (cond_expr_convert_p): Add types_match check when > convert is extension. > * tree-vect-patterns.cc > (gimple_cond_expr_convert_p): Adjust comments. > (vect_recog_cond_expr_convert_pattern): Ditto. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/pr104551.c: New test. > --- > gcc/match.pd | 8 +++++--- > gcc/testsuite/gcc.target/i386/pr104551.c | 24 ++++++++++++++++++++++++ > gcc/tree-vect-patterns.cc | 6 ++++-- > 3 files changed, 33 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr104551.c > > diff --git a/gcc/match.pd b/gcc/match.pd > index 05a10ab6bfd..8e80b9f1576 100644 > --- a/gcc/match.pd > +++ b/gcc/match.pd > @@ -7692,11 +7692,13 @@ and, > (if (INTEGRAL_TYPE_P (type) > && INTEGRAL_TYPE_P (TREE_TYPE (@2)) > && INTEGRAL_TYPE_P (TREE_TYPE (@0)) > - && INTEGRAL_TYPE_P (TREE_TYPE (@3)) > && TYPE_PRECISION (type) != TYPE_PRECISION (TREE_TYPE (@0)) > && TYPE_PRECISION (TREE_TYPE (@0)) > == TYPE_PRECISION (TREE_TYPE (@2)) > - && TYPE_PRECISION (TREE_TYPE (@0)) > - == TYPE_PRECISION (TREE_TYPE (@3)) > + && (types_match (TREE_TYPE (@2), TREE_TYPE (@3)) > + || ((TYPE_PRECISION (TREE_TYPE (@0)) > + == TYPE_PRECISION (TREE_TYPE (@3))) > + && INTEGRAL_TYPE_P (TREE_TYPE (@3)) > + && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (type))) > && single_use (@4) > && single_use (@5)))) I find this quite unreadable, it looks like if @2 and @3 are treated differently. I think keeping the old 3 lines and just adding && (TYPE_PRECISION (TREE_TYPE (@0)) >= TYPE_PRECISION (type) || (TYPE_UNSIGNED (TREE_TYPE (@2)) == TYPE_UNSIGNED (TREE_TYPE (@3)))) after it ideally with a comment why would be better. Note, if the precision of @0 and type is the same, I think signedness can still differ, no? Jakub
On Wed, Feb 16, 2022 at 10:17 PM Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Wed, Feb 16, 2022 at 05:03:09PM +0800, liuhongt via Gcc-patches wrote: > > > > +(match (cond_expr_convert_p @0 @2 @3 @6) > > > > + (cond (simple_comparison@6 @0 @1) (convert@4 @2) (convert@5 @3)) > > > > + (if (types_match (TREE_TYPE (@2), TREE_TYPE (@3)) > > > > > > But in principle @2 or @3 could safely differ in sign, you'd then need to ensure > > > to insert sign conversions to @2/@3 to the signedness of @4/@5. > > > > > It turns out differ in sign is not suitable for extension(but ok for truncation), > > because it's zero_extend vs sign_extend. > > > > The patch add types_match check when convert is extension. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu{-m32,}. > > And native Bootstrapped and regtested on CLX. > > > > Ok for trunk? > > > > gcc/ChangeLog: > > > > PR tree-optimization/104551 > > PR tree-optimization/103771 > > * match.pd (cond_expr_convert_p): Add types_match check when > > convert is extension. > > * tree-vect-patterns.cc > > (gimple_cond_expr_convert_p): Adjust comments. > > (vect_recog_cond_expr_convert_pattern): Ditto. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/i386/pr104551.c: New test. > > --- > > gcc/match.pd | 8 +++++--- > > gcc/testsuite/gcc.target/i386/pr104551.c | 24 ++++++++++++++++++++++++ > > gcc/tree-vect-patterns.cc | 6 ++++-- > > 3 files changed, 33 insertions(+), 5 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr104551.c > > > > diff --git a/gcc/match.pd b/gcc/match.pd > > index 05a10ab6bfd..8e80b9f1576 100644 > > --- a/gcc/match.pd > > +++ b/gcc/match.pd > > @@ -7692,11 +7692,13 @@ and, > > (if (INTEGRAL_TYPE_P (type) > > && INTEGRAL_TYPE_P (TREE_TYPE (@2)) > > && INTEGRAL_TYPE_P (TREE_TYPE (@0)) > > - && INTEGRAL_TYPE_P (TREE_TYPE (@3)) > > && TYPE_PRECISION (type) != TYPE_PRECISION (TREE_TYPE (@0)) > > && TYPE_PRECISION (TREE_TYPE (@0)) > > == TYPE_PRECISION (TREE_TYPE (@2)) > > - && TYPE_PRECISION (TREE_TYPE (@0)) > > - == TYPE_PRECISION (TREE_TYPE (@3)) > > + && (types_match (TREE_TYPE (@2), TREE_TYPE (@3)) > > + || ((TYPE_PRECISION (TREE_TYPE (@0)) > > + == TYPE_PRECISION (TREE_TYPE (@3))) > > + && INTEGRAL_TYPE_P (TREE_TYPE (@3)) > > + && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (type))) > > && single_use (@4) > > && single_use (@5)))) > > I find this quite unreadable, it looks like if @2 and @3 are treated > differently. I think keeping the old 3 lines and just adding > && (TYPE_PRECISION (TREE_TYPE (@0)) >= TYPE_PRECISION (type) > || (TYPE_UNSIGNED (TREE_TYPE (@2)) > == TYPE_UNSIGNED (TREE_TYPE (@3)))) Yes, good idea. > after it ideally with a comment why would be better. > Note, if the precision of @0 and type is the same, I think signedness can > still differ, no? We have TYPE_PRECISION (type) != TYPE_PRECISION (TREE_TYPE (@0)). > > Jakub >
diff --git a/gcc/match.pd b/gcc/match.pd index 05a10ab6bfd..8e80b9f1576 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -7692,11 +7692,13 @@ and, (if (INTEGRAL_TYPE_P (type) && INTEGRAL_TYPE_P (TREE_TYPE (@2)) && INTEGRAL_TYPE_P (TREE_TYPE (@0)) - && INTEGRAL_TYPE_P (TREE_TYPE (@3)) && TYPE_PRECISION (type) != TYPE_PRECISION (TREE_TYPE (@0)) && TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (TREE_TYPE (@2)) - && TYPE_PRECISION (TREE_TYPE (@0)) - == TYPE_PRECISION (TREE_TYPE (@3)) + && (types_match (TREE_TYPE (@2), TREE_TYPE (@3)) + || ((TYPE_PRECISION (TREE_TYPE (@0)) + == TYPE_PRECISION (TREE_TYPE (@3))) + && INTEGRAL_TYPE_P (TREE_TYPE (@3)) + && TYPE_PRECISION (TREE_TYPE (@3)) > TYPE_PRECISION (type))) && single_use (@4) && single_use (@5)))) diff --git a/gcc/testsuite/gcc.target/i386/pr104551.c b/gcc/testsuite/gcc.target/i386/pr104551.c new file mode 100644 index 00000000000..6300f25c0d5 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr104551.c @@ -0,0 +1,24 @@ +/* { dg-do run } */ +/* { dg-options "-O3 -mavx2" } */ +/* { dg-require-effective-target avx2 } */ + +unsigned int +__attribute__((noipa)) +test(unsigned int a, unsigned char p[16]) { + unsigned int res = 0; + for (unsigned b = 0; b < a; b += 1) + res = p[b] ? p[b] : (char) b; + return res; +} + +int main () +{ + unsigned int a = 16U; + unsigned char p[16]; + for (int i = 0; i != 16; i++) + p[i] = (unsigned char)128; + unsigned int res = test (a, p); + if (res != 128) + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc index a8f96d59643..217bdfd7045 100644 --- a/gcc/tree-vect-patterns.cc +++ b/gcc/tree-vect-patterns.cc @@ -929,8 +929,10 @@ vect_reassociating_reduction_p (vec_info *vinfo, with conditions: 1) @1, @2, c, d, a, b are all integral type. 2) There's single_use for both @1 and @2. - 3) a, c and d have same precision. + 3) a, c have same precision. 4) c and @1 have different precision. + 5) c, d are the same type or they can differ in sign when convert is + truncation. record a and c and d and @3. */ @@ -952,7 +954,7 @@ extern bool gimple_cond_expr_convert_p (tree, tree*, tree (*)(tree)); TYPE_PRECISION (TYPE_E) != TYPE_PRECISION (TYPE_CD); TYPE_PRECISION (TYPE_AB) == TYPE_PRECISION (TYPE_CD); single_use of op_true and op_false. - TYPE_AB could differ in sign. + TYPE_AB could differ in sign when (TYPE_E) A is a truncation. Input: