Re: [PATCH] [AVX512] Fix ICE: Convert integer mask to vector in ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp [PR98537]

[Hongtao Liu <crazylht@gmail.com>]

ping.

On Thu, Jan 7, 2021 at 1:22 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Wed, Jan 6, 2021 at 10:39 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Wed, Jan 06, 2021 at 02:49:13PM +0800, Hongtao Liu wrote:
> > >   ix86_expand_fp_vec_cmp/ix86_expand_int_vec_cmp are used by vec_cmpmn
> > > for vector comparison to vector mask, but ix86_expand_sse_cmp(which is
> > > called in upper 2 functions.) may return integer mask whenever integer
> > > mask is available, so convert integer mask back to vector mask if
> > > needed.
> > >
> > > gcc/ChangeLog:
> > >
> > >         PR target/98537
> > >         * config/i386/i386-expand.c (ix86_expand_fp_vec_cmp):
> > >         When cmp is integer mask, convert it to vector.
> > >         (ix86_expand_int_vec_cmp): Ditto.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         PR target/98537
> > >         * g++.target/i386/avx512bw-pr98537-1.C: New test.
> > >         * g++.target/i386/avx512vl-pr98537-1.C: New test.
> > >         * g++.target/i386/avx512vl-pr98537-2.C: New test.
> >
> > Do we optimize it then to an AVX/AVX2 comparison if possible?
> >
> When i'm looking at the code, i find there's other places which
> require comparison dest to be vector(i.e. ix86_expand_sse_unpack,
> ix86_expand_mul_widen_evenodd). It's a potential bug.
> So I fix this bug in another way which won't generate an integer mask
> when the comparison dest is required to a vector mask.
>
> Update patch:
>   ix86_expand_sse_cmp/ix86_expand_int_sse_cmp is used for vector
> comparison, considering that avx512 introduces integer mask, but some
> original callers require the dest of comparison to be a vector.
> So add a new parameter vector_mask_p to control the result
> of vector comparison to be vector or not.
>   regtested/bootstrapped on x86_64-linux-gnu{-m32,}.
>
> gcc/ChangeLog:
>
>         PR target/98537
>         * config/i386/i386-expand.c (ix86_expand_sse_cmp): Add a new
>         parameter vector_mask_p to control whether the comparison
>         result should be a vector or not.
>         (ix86_expand_int_sse_cmp): Ditto.
>         (ix86_expand_sse_movcc): cmpmode should be MODE_INT.
>         (ix86_expand_fp_movcc): Allow vector comparison dest as
>         integer mask.
>         (ix86_expand_fp_vcond): Ditto.
>         (ix86_expand_int_vcond): Ditto.
>         (ix86_expand_fp_vec_cmp): Require vector comparison dest as
>         vector.
>         (ix86_expand_int_vec_cmp): Ditto.
>         (ix86_expand_sse_unpack): Ditto.
>         (ix86_expand_mul_widen_evenodd): Ditto.
>
> gcc/testsuite/ChangeLog:
>
>         PR target/98537
>         * g++.target/i386/avx512bw-pr98537-1.C: New test.
>         * g++.target/i386/avx512vl-pr98537-1.C: New test.
>         * g++.target/i386/avx512vl-pr98537-2.C: New test.
>
>
> > @@ -4024,8 +4025,18 @@ ix86_expand_fp_vec_cmp (rtx operands[])
> >      cmp = ix86_expand_sse_cmp (operands[0], code, operands[2], operands[3],
> >                                operands[1], operands[2]);
> >
> > -  if (operands[0] != cmp)
> > -    emit_move_insn (operands[0], cmp);
> > +    if (operands[0] != cmp)
> > +    {
> >
> > The indentation of the if above looks wrong.
> > Otherwise LGTM.
> >
> >         Jakub
> >
>
>
> --
> BR,
> Hongtao



-- 
BR,
Hongtao