Re: [PATCH][2/2][i386] STV changes, reg-copy as vector

[Richard Biener <rguenther at suse dot de>]

On Tue, 13 Aug 2019, Uros Bizjak wrote:

> On Tue, Aug 13, 2019 at 2:33 PM Richard Biener <rguenther@suse.de> wrote:
> >
> >
> > The following splits out the change that makes the live-range split /
> > mode change copy pseudos vector mode pseudos.  I've tried reproducing
> > the issue I faced with SImode chains but failed likely because we're
> > never using simple moves in the end while for SImode we can end up
> > with
> >
> > (set (subreg:V4SI (reg:SI 42) 0) (subreg:V4SI (reg:SI 41) 0))
> >
> > which we happily propagate out.  So pursuing this patch independently
> > doesn't make much sense.  Still the main change is in
> > dimode_scalar_chain::make_vector_copies where 'vreg' is now V2DImode
> > instead of DImode and the change to emit_move_insn (vreg, tmp)
> > hints at the change done to the above problematic insn.
> >
> > The rest of the changes deal with some regs already being in
> > the appropriate vector mode.
> >
> > I realize I might have avoided my original issue by emitting not
> > the above reg-reg move but
> >
> >  (set (subreg:V4SI (reg:SI 42) 0)
> >    (vec_merge:V4SI
> >       (vec_duplicate:V4SI (reg:SI 41))
> >       (const_vec [0 0 0 0])
> >       (const_1)))
> >
> > but I didn't try (the full patch contained a testcase for the
> > issue).  Still it seems odd to use DImode pseudos when all uses
> > are wrapped in (subreg:V2DI ...).
> 
> It looks to me that the above is the way to go. make_vector_copies
> creates a scalar pseudo, and all other support functions expect a
> scalar that will be processed with "replace_with_subreg". I think that
> the safest way is to emit the code above for SImode for the
> problematic move insn, and
> 
> > +           emit_move_insn (gen_rtx_SUBREG (V2DImode, vreg 0),
> > +                           gen_rtx_VEC_MERGE (V2DImode,
> > +                                              gen_rtx_VEC_DUPLICATE (V2DImode,
> > +                                                                     tmp),
> > +                                              CONST0_RTX (V2DImode),
> > +                                              GEN_INT (HOST_WIDE_INT_1U)));
> 
> for DImode. This way, you won't need other changes (conditional
> generation of SUBREGs), the above should be enough.
> 
> Oh, and don't use emit_move_insn, emit insn with gen_rtx_SET
> (gen_rtx_SUBREG (...)) should do the trick.

OK, that seems to work on the testcases, which makes splitting out this
patch pointless.  Integrated the change with the full patch.

Thanks,
Richard.