On Mon, Aug 22, 2016 at 11:37:55AM -0500, Segher Boessenkool wrote:
> [ seems this mail never arrived, resending, sorry if it turns out a duplicate 
> ]
> 
> Hi Mike,
> 
> Okay for trunk.  A few comments...
> 
> On Fri, Aug 19, 2016 at 06:17:54PM -0400, Michael Meissner wrote:
> > --- gcc/config/rs6000/rs6000.c      
> > (.../svn+ssh://meiss...@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000)    
> > (revision 239554)
> > +++ gcc/config/rs6000/rs6000.c      (.../gcc/config/rs6000) (working copy)
> > @@ -6692,7 +6692,7 @@ rs6000_expand_vector_init (rtx target, r
> >        if ((int_vector_p || TARGET_VSX) && all_const_zero)
> >     {
> >       /* Zero register.  */
> > -     emit_insn (gen_rtx_SET (target, gen_rtx_XOR (mode, target, target)));
> > +     emit_insn (gen_rtx_SET (target, CONST0_RTX (mode)));
> 
> Can you use emit_move_insn here?  If so, please do.

Ok.

> > +  /* Special case initializing vector int if we are on 64-bit systems with
> > +     direct move or we have the ISA 3.0 instructions.  */
> > +  if (mode == V4SImode  && VECTOR_MEM_VSX_P (V4SImode)
> > +      && TARGET_DIRECT_MOVE_64BIT)
> >      {
> > -      emit_insn (gen_vsx_splat_v4si (target, XVECEXP (vals, 0, 0)));
> > -      return;
> > +      if (all_same)
> > +   {
> > +     rtx element0 = XVECEXP (vals, 0, 0);
> > +     if (MEM_P (element0))
> > +       element0 = rs6000_address_for_fpconvert (element0);
> > +     else if (!REG_P (element0))
> > +       element0 = force_reg (SImode, element0);
> 
> You can call force_reg if REG_P holds as well (it immediately returns
> that reg itself).

Ok.

> > +static void
> > +rs6000_split_v4si_init_di_reg (rtx dest, rtx si1, rtx si2, rtx tmp)
> > +{
> > +  const unsigned HOST_WIDE_INT mask_32bit = HOST_WIDE_INT_C (0xffffffff);
> 
> Does using that macro buy us anything?  Won't the plain number work just
> as well?

I would imagine you don't want to use a bare 0xffffffff if the compiler is
being built in a 32-bit environment.  Also, using mask_32bit as a const allowed
me to code the following lines without breaking them into smaller lines due to
the archic 79 character column limit.

> > +      /* Generate RLDIC.  */
> > +      rtx si1_di = gen_rtx_REG (DImode, regno_or_subregno (si1));
> > +      rtx shift_rtx = gen_rtx_ASHIFT (DImode, si1_di, GEN_INT (32));
> > +      rtx mask_rtx = GEN_INT (mask_32bit << 32);
> > +      rtx and_rtx = gen_rtx_AND (DImode, shift_rtx, mask_rtx);
> > +      gcc_assert (!reg_overlap_mentioned_p (dest, si1));
> > +      emit_insn (gen_rtx_SET (dest, and_rtx));
> 
> Maybe gen_rotldi3_mask (after taking the "*" off of that)?  Is that too
> unfriendly to use?  We could add another helper.

The problem is rotld3_mask takes a match_operator, and you pretty much would
have to construct the AND part of the expression, it is kind of useless to call
the generator function (and having to first create the AND insn, and then have
the generator function only do GET_CODE on the part, and ignore it) means an
extra insn is created that will add space until it is garbage collected.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797

Reply via email to