Re: V2 [PATCH] x86: Check mode of pseudo register push

[Uros Bizjak via Gcc-patches]

On Sun, Dec 6, 2020 at 8:11 PM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> On Sun, Dec 6, 2020 at 10:59 AM Uros Bizjak <ubiz...@gmail.com> wrote:
> >
> > On Sun, Dec 6, 2020 at 7:51 PM H.J. Lu <hjl.to...@gmail.com> wrote:
> > >
> > > commit 266f44a91c0c9705d3d18e82d7c5bab32927a18f
> > > Author: H.J. Lu <hjl.to...@gmail.com>
> > > Date:   Sun May 17 10:10:34 2020 -0700
> > >
> > >     x86: Allow V1TI vector register pushes
> > >
> > >     Add V1TI vector register push and split it after reload to a sequence
> > >     of:
> > >
> > >     (set (reg:P SP_REG) (plus:P SP_REG) (const_int -8)))
> > >     (set (match_dup 0) (match_dup 1))
> > >
> > > added a pseudo register push check.  But
> > >
> > > (insn 13 12 14 3 (set (mem:SI (pre_dec:SI (reg/f:SI 7 sp)) [0  S4 A32])
> > >         (reg/v:SI 87 [ srclen ])) "x.c":37:16 54 {*pushsi2}
> > >      (expr_list:REG_DEAD (reg/v:SI 87 [ srclen ])
> > >         (expr_list:REG_ARGS_SIZE (const_int 4 [0x4])
> > >             (nil))))
> > >
> > > is not a pseudo register push.  In 64-bit mode, mode of pseudo register
> > > push is TImode.  In 32-bit mode, it is DImode.  Add pseudo register push
> > > mode check to pseudo_reg_set.
> > >
> > > gcc/
> > >
> > >         PR target/98161
> > >         * config/i386/i386-features.c (pseudo_reg_set): Check mode of
> > >         pseudo register push.
> > >
> > > gcc/testsuite/
> > >
> > >         * gcc.target/i386/pr98161.c: New test.
> > > ---
> > >  gcc/config/i386/i386-features.c         |  2 ++
> > >  gcc/testsuite/gcc.target/i386/pr98161.c | 48 +++++++++++++++++++++++++
> > >  2 files changed, 50 insertions(+)
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr98161.c
> > >
> > > diff --git a/gcc/config/i386/i386-features.c 
> > > b/gcc/config/i386/i386-features.c
> > > index ff6676f54f7..8ac11b13ad2 100644
> > > --- a/gcc/config/i386/i386-features.c
> > > +++ b/gcc/config/i386/i386-features.c
> > > @@ -1266,8 +1266,10 @@ pseudo_reg_set (rtx_insn *insn)
> > >      return NULL;
> > >
> > >    /* Check pseudo register push first. */
> > > +  machine_mode mode = TARGET_64BIT ? TImode : DImode;
> > >    if (REG_P (SET_SRC (set))
> > >        && !HARD_REGISTER_P (SET_SRC (set))
> > > +      && GET_MODE (SET_DEST (set)) == mode
> > >        && push_operand (SET_DEST (set), GET_MODE (SET_DEST (set))))
> >
> > && push_operand (SET_DEST (set), mode)
> >
> > instead?
> >
> > push_operand checks the mode by itself:
> >
> > --q--
> > int
> > push_operand (rtx op, machine_mode mode)
> > {
> >   if (!MEM_P (op))
> >     return 0;
> >
> >   if (mode != VOIDmode && GET_MODE (op) != mode)
> >     return 0;
> >   ...
> > -/q-
> >
> > Uros.
>
> Fixed.   Here is the updated patch.  OK for master?

LGTM.

Thanks,
Uros.