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
>
--
BR,
Hongtao
