> -----Original Message-----
> From: Ramana Radhakrishnan [mailto:ramana.gcc@googlemail.com]
> Sent: Friday, June 27, 2014 9:22 AM
> To: Bin Cheng
> Cc: Richard Earnshaw; gcc-patches
> Subject: Re: [PATCH ARM] Improve ARM memset inlining
> 
> On Tue, May 6, 2014 at 5:59 AM, bin.cheng <bin.cheng@arm.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> >> owner@gcc.gnu.org] On Behalf Of bin.cheng
> >> Sent: Monday, May 05, 2014 3:21 PM
> >> To: Richard Earnshaw
> >> Cc: gcc-patches@gcc.gnu.org
> >> Subject: RE: [PATCH ARM] Improve ARM memset inlining
> >>
> >> Hi Richard,  Thanks for reviewing.  I embedded answers to your
> >> comments, also updated the patch.
> >>
> >> > -----Original Message-----
> >> > From: Richard Earnshaw
> >> > Sent: Friday, May 02, 2014 10:00 PM
> >> > To: Bin Cheng
> >> > Cc: gcc-patches@gcc.gnu.org
> >> > Subject: Re: [PATCH ARM] Improve ARM memset inlining
> >> >
> >> > On 30/04/14 03:52, bin.cheng wrote:
> >> > > Hi,
> >> > > This patch expands small memset calls into direct memory set
> >> > > instructions by introducing "setmemsi" pattern.  For processors
> >> > > without NEON support, it expands memset using general store
> >> > > instruction.  For example, strd for 4-bytes aligned addresses.
> >> > > For processors with NEON support, it expands memset using neon
> >> > > instructions like vstr and miscellaneous vst1.* instructions for
> >> > > both
> >> aligned
> >> > and unaligned cases.
> >> > >
> >> > > This patch depends on
> >> > > http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01923.html otherwise
> >> > > vst1.64 will be generated for 32-bit aligned memory unit.
> >> > >
> >> > > There is also one leftover work of this patch:  Since vst1.*
> >> > > instructions only support post-increment addressing mode, the
> >> > > inlined memset for unaligned neon cases should be like:
> >> > >   vmov.i32   q8, #...
> >> > >   vst1.8     {q8}, [r3]!
> >> > >   vst1.8     {q8}, [r3]!
> >> > >   vst1.8     {q8}, [r3]!
> >> > >   vst1.8     {q8}, [r3]
> >> >
> >> > Other than for zero, I'd expect the vmov to be vmov.i8 to move an
> >> arbitrary
> >> I just used vmov.i32 as an example.  The element size is actually
> > calculated by
> >> function neon_valid_immediate which works as expected I think.
> >>
> >> > byte value into all lanes in a vector.  After that, if the
> >> > alignment is
> >> known to
> >> > be more than 8-bit, I'd expect the vst1 instructions (with the
> >> > exception
> >> of the
> >> > last store if the length is not a multiple of the alignment) to use
> >> >
> >> >     vst1.<align> {reg}, [addr-reg :<align>]!
> >> >
> >> > Hence, for 16-bit aligned data, we want
> >> >
> >> >     vst1.16 {q8}, [r3:16]!
> >> Did I miss something important?  It seems to me the explicit
> >> alignment
> > notes
> >> supported are 64/128/256.  So what do you mean by 16 bits alignment
> here?
> >>
> >> >
> >> > > But for now, gcc can't do this and below code is generated:
> >> > >   vmov.i32   q8, #...
> >> > >   vst1.8     {q8}, [r3]
> >> > >   add        r2,   r3,  #16
> >> > >   add        r3,   r2,  #16
> >> > >   vst1.8     {q8}, [r2]
> >> > >   vst1.8     {q8}, [r3]
> >> > >   add        r2,   r3,  #16
> >> > >   vst1.8     {q8}, [r2]
> >> > >
> >> > > I investigated this issue.  The root cause lies in rtx cost
> >> > > returned by ARM backend.  Anyway, I think this is another issue
> >> > > and should be fixed in separated patch.
> 
> Ok looks like Charles B from Linaro has run into the same thing and has some
> fixes to suggest in costs.
> 
> >> > >
> >> > > Bootstrap and reg-test on cortex-a15, with or without neon support.
> >> > > Is it OK?
> >> > >
> >> >
> >> > Some more comments inline.
> >> >
> >> > > Thanks,
> >> > > bin
> >> > >
> >> > >
> >> > > 2014-04-29  Bin Cheng  <bin.cheng@arm.com>
> >> > >
> >> > >   PR target/55701
> >> > >   * config/arm/arm.md (setmem): New pattern.
> >> > >   * config/arm/arm-protos.h (struct tune_params): New field.
> >> > >   (arm_gen_setmem): New prototype.
> >> > >   * config/arm/arm.c (arm_slowmul_tune): Initialize new field.
> >> > >   (arm_fastmul_tune, arm_strongarm_tune, arm_xscale_tune): Ditto.
> >> > >   (arm_9e_tune, arm_v6t2_tune, arm_cortex_tune): Ditto.
> >> > >   (arm_cortex_a8_tune, arm_cortex_a7_tune): Ditto.
> >> > >   (arm_cortex_a15_tune, arm_cortex_a53_tune): Ditto.
> >> > >   (arm_cortex_a57_tune, arm_cortex_a5_tune): Ditto.
> >> > >   (arm_cortex_a9_tune, arm_cortex_a12_tune): Ditto.
> >> > >   (arm_v7m_tune, arm_v6m_tune, arm_fa726te_tune): Ditto.
> >> > >   (arm_const_inline_cost): New function.
> >> > >   (arm_block_set_max_insns): New function.
> >> > >   (arm_block_set_straight_profit_p): New function.
> >> > >   (arm_block_set_vect_profit_p): New function.
> >> > >   (arm_block_set_unaligned_vect): New function.
> >> > >   (arm_block_set_aligned_vect): New function.
> >> > >   (arm_block_set_unaligned_straight): New function.
> >> > >   (arm_block_set_aligned_straight): New function.
> >> > >   (arm_block_set_vect, arm_gen_setmem): New functions.
> >> > >
> >> > > gcc/testsuite/ChangeLog
> >> > > 2014-04-29  Bin Cheng  <bin.cheng@arm.com>
> >> > >
> >> > >   PR target/55701
> >> > >   * gcc.target/arm/memset-inline-1.c: New test.
> >> > >   * gcc.target/arm/memset-inline-2.c: New test.
> >> > >   * gcc.target/arm/memset-inline-3.c: New test.
> >> > >   * gcc.target/arm/memset-inline-4.c: New test.
> >> > >   * gcc.target/arm/memset-inline-5.c: New test.
> >> > >   * gcc.target/arm/memset-inline-6.c: New test.
> >> > >   * gcc.target/arm/memset-inline-7.c: New test.
> >> > >   * gcc.target/arm/memset-inline-8.c: New test.
> >> > >   * gcc.target/arm/memset-inline-9.c: New test.
> >> > >
> >> > >
> >> > > j1328-20140429.txt
> >> > >
> >> > >
> >> > > Index: gcc/config/arm/arm.c
> >> > >
> >> >
> >>
> ==========================================================
> >> > =========
> >> > > --- gcc/config/arm/arm.c  (revision 209852)
> >> > > +++ gcc/config/arm/arm.c  (working copy)
> >> > > @@ -1585,10 +1585,11 @@ const struct tune_params
> arm_slowmul_tune
> >> =
> >> > >    true,                                          /* Prefer constant
> >> > pool.  */
> >> > >    arm_default_branch_cost,
> >> > >    false,                                 /* Prefer LDRD/STRD.  */
> >> > > -  {true, true},                                  /* Prefer non short
> >> > circuit.  */
> >> > > -  &arm_default_vec_cost,                        /* Vectorizer costs.
> >> */
> >> > > -  false,                                        /* Prefer Neon for
> >> 64-bits bitops.  */
> >> > > -  false, false                                  /* Prefer 32-bit
> >> encodings.  */
> >> > > +  {true, true},                          /* Prefer non short circuit.
> >> */
> >> > > +  &arm_default_vec_cost,                /* Vectorizer costs.  */
> >> > > +  false,                                /* Prefer Neon for 64-bits
> >> bitops.  */
> >> > > +  false, false,                         /* Prefer 32-bit encodings.
> > */
> >> > > +  false                                 /* Prefer Neon for stringops.
> >> */
> >> > >  };
> >> > >
> >> >
> >> > Please make sure that all the white space before the comments is
> >> > using
> >> TAB,
> >> > not spaces.  Similarly for the other tables.
> >> Fixed.
> >>
> >> >
> >> > > @@ -16788,6 +16806,14 @@ arm_const_double_inline_cost (rtx val)
> >> > >                         NULL_RTX, NULL_RTX, 0, 0));  }
> >> > >
> >> > > +/* Cost of loading a SImode constant.  */ static inline int
> >> > > +arm_const_inline_cost (rtx val) {
> >> > > +  return arm_gen_constant (SET, SImode, NULL_RTX, INTVAL (val),
> >> > > +                           NULL_RTX, NULL_RTX, 0, 0); }
> >> > > +
> >> >
> >> > This could be used more widely if you passed the SET in as a
> >> > parameter (there are cases in arm_new_rtx_cost that could use it, for
> example).
> >> > Also, you want to enable sub-targets (only once you can't create
> >> > new pseudos is that not safe), so the penultimate argument in the
> >> > call to arm_gen_constant should be 1.
> >> Fixed.
> 
> 
> 
> >>
> >> >
> >> > >  /* Return true if it is worthwhile to split a 64-bit constant
> >> > > into
> > two
> >> > >     32-bit operations.  This is the case if optimizing for size, or
> >> > >     if we have load delay slots, or if one 32-bit part can be
> >> > > done with @@ -31350,6 +31383,504 @@ arm_validize_comparison (rtx
> >> > > *comparison, rtx * op
> >> > >
> >> > >  }
> >> > >
> >> > > +/* Maximum number of instructions to set block of memory.  */
> >> > > +static int arm_block_set_max_insns (void) {
> >> > > +  return (optimize_function_for_size_p (cfun) ? 4 : 8); }
> >> >
> >> > I think the non-size_p alternative should really be a parameter in
> >> > the
> >> per-cpu
> >> > costs table.
> >> Fixed.
> >>
> >> >
> >> > > +
> >> > > +/* Return TRUE if it's profitable to set block of memory for straight
> >> > > +   case.  */
> >> >
> >> > "Straight" is confusing here.  Do you mean non-vectorized?  If so,
> >> > then non_vect might be clearer.
> >> Fixed.
> >>
> >> >
> >> > The arguments should really be documented (see comment below
> about
> >> > align, for example).
> >> Fixed.
> >>
> >> >
> >> > > +static bool
> >> > > +arm_block_set_straight_profit_p (rtx val,
> >> > > +                          unsigned HOST_WIDE_INT length,
> >> > > +                          unsigned HOST_WIDE_INT align,
> >> > > +                          bool unaligned_p, bool use_strd_p) {
> >> > > +  int num = 0;
> >> > > +  /* For leftovers in bytes of 0-7, we can set the memory block using
> >> > > +     strb/strh/str with minimum instruction number.  */
> >> > > +  int leftover[8] = {0, 1, 1, 2, 1, 2, 2, 3};
> >> >
> >> > This should be marked const.
> >> Fixed.
> >>
> >> >
> >> > > +
> >> > > +  if (unaligned_p)
> >> > > +    {
> >> > > +      num = arm_const_inline_cost (val);
> >> > > +      num += length / align + length % align;
> >> >
> >> > Isn't align in bits here, when you really want it in bytes?
> >> All alignments are in bytes starting from pattern "setmem".
> >>
> >> >
> >> > What if align > 4 bytes?
> >> Then it's the "!unaligned_p" case and handled by other arms of this
> >> if statement.
> >>
> >> >
> >> > > +    }
> >> > > +  else if (use_strd_p)
> >> > > +    {
> >> > > +      num = arm_const_double_inline_cost (val);
> >> > > +      num += (length >> 3) + leftover[length & 7];
> >> > > +    }
> >> > > +  else
> >> > > +    {
> >> > > +      num = arm_const_inline_cost (val);
> >> > > +      num += (length >> 2) + leftover[length & 3];
> >> > > +    }
> >> > > +
> >> > > +  /* We may be able to combine last pair STRH/STRB into a single STR
> >> > > +     by shifting one byte back.  */  if (unaligned_access &&
> >> > > + length
> >> > > + > 3 && (length & 3) == 3)
> >> > > +    num--;
> >> > > +
> >> > > +  return (num <= arm_block_set_max_insns ()); }
> >> > > +
> >> > > +/* Return TRUE if it's profitable to set block of memory for
> >> > > +vector case.  */ static bool arm_block_set_vect_profit_p
> >> > > +(unsigned HOST_WIDE_INT length,
> >> > > +                      unsigned HOST_WIDE_INT align
> >> > ATTRIBUTE_UNUSED,
> >> > > +                      bool unaligned_p, enum machine_mode mode)
> >> >
> >> > I'm not sure what you mean by unaligned here.  Again, documenting
> >> > the arguments might help.
> >> Fixed.
> >>
> >> >
> >> > > +{
> >> > > +  int num;
> >> > > +  unsigned int nelt = GET_MODE_NUNITS (mode);
> >> > > +
> >> > > +  /* Num of instruction loading constant value.  */
> >> >
> >> > Use either "Number" or, in this case, simply drop that bit and write:
> >> >   /* Instruction loading constant value.  */
> >> Fixed.
> >>
> >> >
> >> > > +  num = 1;
> >> > > +  /* Num of store instructions.  */
> >> >
> >> > Likewise.
> >> >
> >> > > +  num += (length + nelt - 1) / nelt;
> >> > > +  /* Num of address adjusting instructions.  */
> >> >
> >> > Can't we work on the premise that the address adjusting
> >> > instructions will
> >> be
> >> > merged into the stores?  I know you said that they currently do
> >> > not, but that's not a problem that this bit of code should have to
> >> > worry
> > about.
> >> Fixed.
> >>
> >> >
> >> > > +  if (unaligned_p)
> >> > > +    /* For unaligned case, it's one less than the store instructions.
> >> */
> >> > > +    num += (length + nelt - 1) / nelt - 1;  else if ((length &
> >> > > + 3) !=
> >> > > + 0)
> >> > > +    /* For aligned case, it's one if bytes leftover can only be
> > stored
> >> > > +       by mis-aligned store instruction.  */
> >> > > +    num++;
> >> > > +
> >> > > +  /* Store the first 16 bytes using vst1:v16qi for the aligned case.
> >> > > + */  if (!unaligned_p && mode == V16QImode)
> >> > > +    num--;
> >> > > +
> >> > > +  return (num <= arm_block_set_max_insns ()); }
> >> > > +
> >> > > +/* Set a block of memory using vectorization instructions for the
> >> > > +   unaligned case.  We fill the first LENGTH bytes of the memory
> >> > > +   area starting from DSTBASE with byte constant VALUE.  ALIGN is
> >> > > +   the alignment requirement of memory.  */
> >> >
> >> > What's the return value mean?
> >> Documented.
> >>
> >> >
> >> > > +static bool
> >> > > +arm_block_set_unaligned_vect (rtx dstbase,
> >> > > +                       unsigned HOST_WIDE_INT length,
> >> > > +                       unsigned HOST_WIDE_INT value,
> >> > > +                       unsigned HOST_WIDE_INT align) {
> >> > > +  unsigned int i = 0, j = 0, nelt_v16, nelt_v8, nelt_mode;
> >> >
> >> > Don't mix initialized declarations with unitialized ones on the
> >> > same
> > line.
> >> You
> >> > don't appear to use either I or J until their first use in the loop
> >> control below,
> >> > so why initialize them here?
> >> Fixed.
> >>
> >> >
> >> > > +  rtx dst, mem;
> >> > > +  rtx val_elt, val_vec, reg;
> >> > > +  rtx rval[MAX_VECT_LEN];
> >> > > +  rtx (*gen_func) (rtx, rtx);
> >> > > +  enum machine_mode mode;
> >> > > +  unsigned HOST_WIDE_INT v = value;
> >> > > +
> >> > > +  gcc_assert ((align & 0x3) != 0);
> >> > > +  nelt_v8 = GET_MODE_NUNITS (V8QImode);
> >> > > +  nelt_v16 = GET_MODE_NUNITS (V16QImode);  if (length >=
> nelt_v16)
> >> > > +    {
> >> > > +      mode = V16QImode;
> >> > > +      gen_func = gen_movmisalignv16qi;
> >> > > +    }
> >> > > +  else
> >> > > +    {
> >> > > +      mode = V8QImode;
> >> > > +      gen_func = gen_movmisalignv8qi;
> >> > > +    }
> >> > > +  nelt_mode = GET_MODE_NUNITS (mode);  gcc_assert (length >=
> >> > > + nelt_mode);
> >> > > +  /* Skip if it isn't profitable.  */  if
> >> > > + (!arm_block_set_vect_profit_p (length, align, true, mode))
> >> > > +    return false;
> >> > > +
> >> > > +  dst = copy_addr_to_reg (XEXP (dstbase, 0));  mem =
> >> > > + adjust_automodify_address (dstbase, mode, dst, 0);
> >> > > +
> >> > > +  v = sext_hwi (v, BITS_PER_WORD);  val_elt = GEN_INT (v);  for
> >> > > + (; j < nelt_mode; j++)
> >> > > +    rval[j] = val_elt;
> >> >
> >> > Is this the first use of J?  If so, initialize it here.
> >> >
> >> > > +
> >> > > +  reg = gen_reg_rtx (mode);
> >> > > +  val_vec = gen_rtx_CONST_VECTOR (mode, gen_rtvec_v
> (nelt_mode,
> >> > > + rval));
> >> > > +  /* Emit instruction loading the constant value.  */
> >> > > + emit_move_insn (reg, val_vec);
> >> > > +
> >> > > +  /* Handle nelt_mode bytes in a vector.  */  for (; (i +
> >> > > + nelt_mode <= length); i += nelt_mode)
> >> >
> >> > Similarly for I.
> >> >
> >> > > +    {
> >> > > +      emit_insn ((*gen_func) (mem, reg));
> >> > > +      if (i + 2 * nelt_mode <= length) emit_insn (gen_add2_insn
> >> > > + (dst, GEN_INT (nelt_mode)));
> >> > > +    }
> >> > > +
> >> > > +  if (i + nelt_v8 <= length)
> >> > > +    gcc_assert (mode == V16QImode);
> >> >
> >> > Why not drop the if and write:
> >> >
> >> >      gcc_assert ((i + nelt_v8) > length || mode == V16QImode);
> >> Fixed.
> >>
> >> >
> >> > > +
> >> > > +  /* Handle (8, 16) bytes leftover.  */  if (i + nelt_v8 <
> >> > > + length)
> >> >
> >> > Your assertion above checked <=, but here you use <.  Is that correct?
> >> Yes, it is.  For case "==", it means we have nelt_v8 bytes leftover,
> >> which
> > will
> >> be handled by the last branch of if statement.
> >>
> >> >
> >> > > +    {
> >> > > +      emit_insn (gen_add2_insn (dst, GEN_INT (length - i)));
> >> > > +      /* We are shifting bytes back, set the alignment accordingly.
> > */
> >> > > +      if ((length & 1) != 0 && align >= 2) set_mem_align (mem,
> >> > > + BITS_PER_UNIT);
> >> > > +
> >> > > +      emit_insn (gen_movmisalignv16qi (mem, reg));
> >> > > +    }
> >> > > +  /* Handle (0, 8] bytes leftover.  */  else if (i < length && i
> >> > > + + nelt_v8 >= length)
> >> > > +    {
> >> > > +      if (mode == V16QImode)
> >> > > + {
> >> > > +   reg = gen_lowpart (V8QImode, reg);
> >> > > +   mem = adjust_automodify_address (dstbase, V8QImode, dst, 0);
> >> > > + }
> >> > > +      emit_insn (gen_add2_insn (dst, GEN_INT ((length - i)
> >> > > +                                       + (nelt_mode - nelt_v8))));
> >> > > +      /* We are shifting bytes back, set the alignment accordingly.
> > */
> >> > > +      if ((length & 1) != 0 && align >= 2) set_mem_align (mem,
> >> > > + BITS_PER_UNIT);
> >> > > +
> >> > > +      emit_insn (gen_movmisalignv8qi (mem, reg));
> >> > > +    }
> >> > > +
> >> > > +  return true;
> >> > > +}
> >> > > +
> >> > > +/* Set a block of memory using vectorization instructions for the
> >> > > +   aligned case.  We fill the first LENGTH bytes of the memory area
> >> > > +   starting from DSTBASE with byte constant VALUE.  ALIGN is the
> >> > > +   alignment requirement of memory.  */
> >> >
> >> > See all the comments above for the unaligend case.
> >> Fixed accordingly.
> >>
> >> >
> >> > > +static bool
> >> > > +arm_block_set_aligned_vect (rtx dstbase,
> >> > > +                     unsigned HOST_WIDE_INT length,
> >> > > +                     unsigned HOST_WIDE_INT value,
> >> > > +                     unsigned HOST_WIDE_INT align) {
> >> > > +  unsigned int i = 0, j = 0, nelt_v8, nelt_v16, nelt_mode;
> >> > > +  rtx dst, addr, mem;
> >> > > +  rtx val_elt, val_vec, reg;
> >> > > +  rtx rval[MAX_VECT_LEN];
> >> > > +  enum machine_mode mode;
> >> > > +  unsigned HOST_WIDE_INT v = value;
> >> > > +
> >> > > +  gcc_assert ((align & 0x3) == 0);
> >> > > +  nelt_v8 = GET_MODE_NUNITS (V8QImode);
> >> > > +  nelt_v16 = GET_MODE_NUNITS (V16QImode);  if (length >=
> >> > > + nelt_v16 && unaligned_access && !BYTES_BIG_ENDIAN)
> >> > > +    mode = V16QImode;
> >> > > +  else
> >> > > +    mode = V8QImode;
> >> > > +
> >> > > +  nelt_mode = GET_MODE_NUNITS (mode);  gcc_assert (length >=
> >> > > + nelt_mode);
> >> > > +  /* Skip if it isn't profitable.  */  if
> >> > > + (!arm_block_set_vect_profit_p (length, align, false, mode))
> >> > > +    return false;
> >> > > +
> >> > > +  dst = copy_addr_to_reg (XEXP (dstbase, 0));
> >> > > +
> >> > > +  v = sext_hwi (v, BITS_PER_WORD);  val_elt = GEN_INT (v);  for
> >> > > + (; j < nelt_mode; j++)
> >> > > +    rval[j] = val_elt;
> >> > > +
> >> > > +  reg = gen_reg_rtx (mode);
> >> > > +  val_vec = gen_rtx_CONST_VECTOR (mode, gen_rtvec_v
> (nelt_mode,
> >> > > + rval));
> >> > > +  /* Emit instruction loading the constant value.  */
> >> > > + emit_move_insn (reg, val_vec);
> >> > > +
> >> > > +  /* Handle first 16 bytes specially using vst1:v16qi instruction.
> >> > > +*/
> >> > > +  if (mode == V16QImode)
> >> > > +    {
> >> > > +      mem = adjust_automodify_address (dstbase, mode, dst, 0);
> >> > > +      emit_insn (gen_movmisalignv16qi (mem, reg));
> >> > > +      i += nelt_mode;
> >> > > +      /* Handle (8, 16) bytes leftover using vst1:v16qi again.  */
> >> > > +      if (i + nelt_v8 < length && i + nelt_v16 > length)  {
> >> > > +   emit_insn (gen_add2_insn (dst, GEN_INT (length - nelt_mode)));
> >> > > +   mem = adjust_automodify_address (dstbase, mode, dst, 0);
> >> > > +   /* We are shifting bytes back, set the alignment accordingly.  */
> >> > > +   if ((length & 0x3) == 0)
> >> > > +     set_mem_align (mem, BITS_PER_UNIT * 4);
> >> > > +   else if ((length & 0x1) == 0)
> >> > > +     set_mem_align (mem, BITS_PER_UNIT * 2);
> >> > > +   else
> >> > > +     set_mem_align (mem, BITS_PER_UNIT);
> >> > > +
> >> > > +   emit_insn (gen_movmisalignv16qi (mem, reg));
> >> > > +   return true;
> >> > > + }
> >> > > +      /* Fall through for bytes leftover.  */
> >> > > +      mode = V8QImode;
> >> > > +      nelt_mode = GET_MODE_NUNITS (mode);
> >> > > +      reg = gen_lowpart (V8QImode, reg);
> >> > > +    }
> >> > > +
> >> > > +  /* Handle 8 bytes in a vector.  */  for (; (i + nelt_mode <=
> >> > > + length); i += nelt_mode)
> >> > > +    {
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, mode, addr, i);
> >> > > +      emit_move_insn (mem, reg);
> >> > > +    }
> >> > > +
> >> > > +  /* Handle single word leftover by shifting 4 bytes back.  We can
> >> > > +     use aligned access for this case.  */  if (i +
> >> > > + UNITS_PER_WORD == length)
> >> > > +    {
> >> > > +      addr = plus_constant (Pmode, dst, i - UNITS_PER_WORD);
> >> > > +      mem = adjust_automodify_address (dstbase, mode,
> >> > > +                                addr, i - UNITS_PER_WORD);
> >> > > +      /* We are shifting 4 bytes back, set the alignment accordingly.
> >> */
> >> > > +      if (align > UNITS_PER_WORD) set_mem_align (mem,
> >> > > + BITS_PER_UNIT * UNITS_PER_WORD);
> >> > > +
> >> > > +      emit_move_insn (mem, reg);
> >> > > +    }
> >> > > +  /* Handle (0, 4), (4, 8) bytes leftover by shifting bytes back.
> >> > > +     We have to use unaligned access for this case.  */  else if
> >> > > + (i < length)
> >> > > +    {
> >> > > +      emit_insn (gen_add2_insn (dst, GEN_INT (length - nelt_mode)));
> >> > > +      mem = adjust_automodify_address (dstbase, mode, dst, 0);
> >> > > +      /* We are shifting bytes back, set the alignment accordingly.
> > */
> >> > > +      if ((length & 1) == 0)
> >> > > + set_mem_align (mem, BITS_PER_UNIT * 2);
> >> > > +      else
> >> > > + set_mem_align (mem, BITS_PER_UNIT);
> >> > > +
> >> > > +      emit_insn (gen_movmisalignv8qi (mem, reg));
> >> > > +    }
> >> > > +
> >> > > +  return true;
> >> > > +}
> >> > > +
> >> > > +/* Set a block of memory using plain strh/strb instructions, only
> >> > > +   using instructions allowed by ALIGN on processor.  We fill the
> >> > > +   first LENGTH bytes of the memory area starting from DSTBASE
> >> > > +   with byte constant VALUE.  ALIGN is the alignment requirement
> >> > > +   of memory.  */
> >> > > +static bool
> >> > > +arm_block_set_unaligned_straight (rtx dstbase,
> >> > > +                           unsigned HOST_WIDE_INT length,
> >> > > +                           unsigned HOST_WIDE_INT value,
> >> > > +                           unsigned HOST_WIDE_INT align) {
> >> > > +  unsigned int i;
> >> > > +  rtx dst, addr, mem;
> >> > > +  rtx val_exp, val_reg, reg;
> >> > > +  enum machine_mode mode;
> >> > > +  HOST_WIDE_INT v = value;
> >> > > +
> >> > > +  gcc_assert (align == 1 || align == 2);
> >> > > +
> >> > > +  if (align == 2)
> >> > > +    v |= (value << BITS_PER_UNIT);
> >> > > +
> >> > > +  v = sext_hwi (v, BITS_PER_WORD);  val_exp = GEN_INT (v);
> >> > > +  /* Skip if it isn't profitable.  */  if
> >> > > + (!arm_block_set_straight_profit_p (val_exp, length,
> >> > > +                                 align, true, false))
> >> > > +    return false;
> >> > > +
> >> > > +  dst = copy_addr_to_reg (XEXP (dstbase, 0));  mode = (align == 2 ?
> >> > > + HImode : QImode);  val_reg = force_reg (SImode, val_exp);  reg
> >> > > + = gen_lowpart (mode, val_reg);
> >> > > +
> >> > > +  for (i = 0; (i + GET_MODE_SIZE (mode) <= length); i +=
> >> > > + GET_MODE_SIZE
> >> > (mode))
> >> > > +    {
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, mode, addr, i);
> >> > > +      emit_move_insn (mem, reg);
> >> > > +    }
> >> > > +
> >> > > +  /* Handle single byte leftover.  */  if (i + 1 == length)
> >> > > +    {
> >> > > +      reg = gen_lowpart (QImode, val_reg);
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, QImode, addr, i);
> >> > > +      emit_move_insn (mem, reg);
> >> > > +      i++;
> >> > > +    }
> >> > > +
> >> > > +  gcc_assert (i == length);
> >> > > +  return true;
> >> > > +}
> >> > > +
> >> > > +/* Set a block of memory using plain strd/str/strh/strb instructions,
> >> > > +   to permit unaligned copies on processors which support unaligned
> >> > > +   semantics for those instructions.  We fill the first LENGTH bytes
> >> > > +   of the memory area starting from DSTBASE with byte constant
> VALUE.
> >> > > +   ALIGN is the alignment requirement of memory.  */ static bool
> >> > > +arm_block_set_aligned_straight (rtx dstbase,
> >> > > +                         unsigned HOST_WIDE_INT length,
> >> > > +                         unsigned HOST_WIDE_INT value,
> >> > > +                         unsigned HOST_WIDE_INT align) {
> >> > > +  unsigned int i = 0;
> >> > > +  rtx dst, addr, mem;
> >> > > +  rtx val_exp, val_reg, reg;
> >> > > +  unsigned HOST_WIDE_INT v;
> >> > > +  bool use_strd_p;
> >> > > +
> >> > > +  use_strd_p = (length >= 2 * UNITS_PER_WORD && (align & 3) == 0
> >> > > +         && TARGET_LDRD && current_tune->prefer_ldrd_strd);
> >> > > +
> >> > > +  v = (value | (value << 8) | (value << 16) | (value << 24));
> >> > > + if (length < UNITS_PER_WORD)
> >> > > +    v &= (0xFFFFFFFF >> (UNITS_PER_WORD - length) *
> >> > > + BITS_PER_UNIT);
> >> > > +
> >> > > +  if (use_strd_p)
> >> > > +    v |= (v << BITS_PER_WORD);
> >> > > +  else
> >> > > +    v = sext_hwi (v, BITS_PER_WORD);
> >> > > +
> >> > > +  val_exp = GEN_INT (v);
> >> > > +  /* Skip if it isn't profitable.  */  if
> >> > > + (!arm_block_set_straight_profit_p (val_exp, length,
> >> > > +                                 align, false, use_strd_p))
> >> > > +    {
> >> > > +      /* Try without strd.  */
> >> > > +      v = (v >> BITS_PER_WORD);
> >> > > +      v = sext_hwi (v, BITS_PER_WORD);
> >> > > +      val_exp = GEN_INT (v);
> >> > > +      use_strd_p = false;
> >> > > +      if (!arm_block_set_straight_profit_p (val_exp, length,
> >> > > +                                     align, false, use_strd_p))
> >> > > + return false;
> >> > > +    }
> >> > > +
> >> > > +  dst = copy_addr_to_reg (XEXP (dstbase, 0));
> >> > > +  /* Handle double words using strd if possible.  */  if
> >> > > + (use_strd_p)
> >> > > +    {
> >> > > +      val_reg = force_reg (DImode, val_exp);
> >> > > +      reg = val_reg;
> >> > > +      for (; (i + 8 <= length); i += 8) {
> >> > > +   addr = plus_constant (Pmode, dst, i);
> >> > > +   mem = adjust_automodify_address (dstbase, DImode, addr, i);
> >> > > +   emit_move_insn (mem, reg);
> >> > > + }
> >> > > +    }
> >> > > +  else
> >> > > +    val_reg = force_reg (SImode, val_exp);
> >> > > +
> >> > > +  /* Handle words.  */
> >> > > +  reg = (use_strd_p ? gen_lowpart (SImode, val_reg) : val_reg);
> >> > > + for (; (i + 4 <= length); i += 4)
> >> > > +    {
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, SImode, addr, i);
> >> > > +      if ((align & 3) == 0)
> >> > > + emit_move_insn (mem, reg);
> >> > > +      else
> >> > > + emit_insn (gen_unaligned_storesi (mem, reg));
> >> > > +    }
> >> > > +
> >> > > +  /* Merge last pair of STRH and STRB into a STR if possible.
> >> > > + */ if (unaligned_access && i > 0 && (i + 3) == length)
> >> > > +    {
> >> > > +      addr = plus_constant (Pmode, dst, i - 1);
> >> > > +      mem = adjust_automodify_address (dstbase, SImode, addr, i - 1);
> >> > > +      /* We are shifting one byte back, set the alignment
> > accordingly.
> >> */
> >> > > +      if ((align & 1) == 0)
> >> > > + set_mem_align (mem, BITS_PER_UNIT);
> >> > > +
> >> > > +      /* Most likely this is an unaligned access, and we can't
> >> > > + tell
> > at
> >> > > +  compilation time.  */
> >> > > +      emit_insn (gen_unaligned_storesi (mem, reg));
> >> > > +      return true;
> >> > > +    }
> >> > > +
> >> > > +  /* Handle half word leftover.  */  if (i + 2 <= length)
> >> > > +    {
> >> > > +      reg = gen_lowpart (HImode, val_reg);
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, HImode, addr, i);
> >> > > +      if ((align & 1) == 0)
> >> > > + emit_move_insn (mem, reg);
> >> > > +      else
> >> > > + emit_insn (gen_unaligned_storehi (mem, reg));
> >> > > +
> >> > > +      i += 2;
> >> > > +    }
> >> > > +
> >> > > +  /* Handle single byte leftover.  */  if (i + 1 == length)
> >> > > +    {
> >> > > +      reg = gen_lowpart (QImode, val_reg);
> >> > > +      addr = plus_constant (Pmode, dst, i);
> >> > > +      mem = adjust_automodify_address (dstbase, QImode, addr, i);
> >> > > +      emit_move_insn (mem, reg);
> >> > > +    }
> >> > > +
> >> > > +  return true;
> >> > > +}
> >> > > +
> >> > > +/* Set a block of memory using vectorization instructions for both
> >> > > +   aligned and unaligned cases.  We fill the first LENGTH bytes of
> >> > > +   the memory area starting from DSTBASE with byte constant VALUE.
> >> > > +   ALIGN is the alignment requirement of memory.  */ static bool
> >> > > +arm_block_set_vect (rtx dstbase,
> >> > > +             unsigned HOST_WIDE_INT length,
> >> > > +             unsigned HOST_WIDE_INT value,
> >> > > +             unsigned HOST_WIDE_INT align) {
> >> > > +  /* Check whether we need to use unaligned store instruction.
> >> > > +*/
> >> > > +  if (((align & 3) != 0 || (length & 3) != 0)
> >> > > +      /* Check whether unaligned store instruction is available.  */
> >> > > +      && (!unaligned_access || BYTES_BIG_ENDIAN))
> >> > > +    return false;
> >> >
> >> > Huh!  vst1.8 can work for unaligned accesses even when hw alignment
> >> > checking is strict.
> >> Emm, All movmisalign patterns are guarded by " !BYTES_BIG_ENDIAN &&
> >> unaligned_access", vst1.8 instructions  can't be recognized now in
> >> this
> > way.
> >> I agree that it's too strict, but that's another problem I think.
> 
> That was introduced to "fix up" the issue with another test IIRC.
> That's probably not related to this particular patch. It's the other thread that's
> been ongoing with Maciej, so let's continue it there.
> 
> >>
> >> >
> >> > > +
> >> > > +  if ((align & 3) == 0)
> >> > > +    return arm_block_set_aligned_vect (dstbase, length, value,
> >> > > +align);
> >> > > +  else
> >> > > +    return arm_block_set_unaligned_vect (dstbase, length, value,
> >> > > +align); }
> >> > > +
> >> > > +/* Expand string store operation.  Firstly we try to do that by using
> >> > > +   vectorization instructions, then try with ARM unaligned access and
> >> > > +   double-word store if profitable.  OPERANDS[0] is the destination,
> >> > > +   OPERANDS[1] is the number of bytes, operands[2] is the value to
> >> > > +   initialize the memory, OPERANDS[3] is the known alignment of the
> >> > > +   destination.  */
> >> > > +bool
> >> > > +arm_gen_setmem (rtx *operands)
> >> > > +{
> >> > > +  rtx dstbase = operands[0];
> >> > > +  unsigned HOST_WIDE_INT length;
> >> > > +  unsigned HOST_WIDE_INT value;
> >> > > +  unsigned HOST_WIDE_INT align;
> >> > > +
> >> > > +  if (!CONST_INT_P (operands[2]) || !CONST_INT_P (operands[1]))
> >> > > +    return false;
> >> > > +
> >> > > +  length = UINTVAL (operands[1]);  if (length > 64)
> >> > > +    return false;
> >> > > +
> >> > > +  value = (UINTVAL (operands[2]) & 0xFF);  align = UINTVAL
> >> > > + (operands[3]);  if (TARGET_NEON && length >= 8
> >> > > +      && current_tune->string_ops_prefer_neon
> >> > > +      && arm_block_set_vect (dstbase, length, value, align))
> >> > > +    return true;
> >> > > +
> >> > > +  if (!unaligned_access && (align & 3) != 0)
> >> > > +    return arm_block_set_unaligned_straight (dstbase, length,
> >> > > + value, align);
> >> > > +
> >> > > +  return arm_block_set_aligned_straight (dstbase, length, value,
> >> > > +align); }
> >> > > +
> >> > >  /* Implement the TARGET_ASAN_SHADOW_OFFSET hook.  */
> >> > >
> >> > >  static unsigned HOST_WIDE_INT
> >> > > Index: gcc/config/arm/arm-protos.h
> >> > >
> >> >
> >>
> ==========================================================
> >> > =========
> >> > > --- gcc/config/arm/arm-protos.h   (revision 209852)
> >> > > +++ gcc/config/arm/arm-protos.h   (working copy)
> >> > > @@ -277,6 +277,8 @@ struct tune_params
> >> > >    /* Prefer 32-bit encoding instead of 16-bit encoding where
> >> > > subset of
> >> flags
> >> > >       would be set.  */
> >> > >    bool disparage_partial_flag_setting_t16_encodings;
> >> > > +  /* Prefer to inline string operations like memset by using Neon.
> >> > > + */  bool string_ops_prefer_neon;
> >> > >  };
> >> > >
> >> > >  extern const struct tune_params *current_tune; @@ -289,6 +291,7
> >> > > @@ extern void arm_emit_coreregs_64bit_shift (enum rt  extern
> >> > > bool arm_validize_comparison (rtx *, rtx *, rtx *);  #endif /*
> >> > > RTX_CODE */
> >> > >
> >> > > +extern bool arm_gen_setmem (rtx *);
> >> > >  extern void arm_expand_vec_perm (rtx target, rtx op0, rtx op1,
> >> > > rtx sel);  extern bool arm_expand_vec_perm_const (rtx target, rtx
> >> > > op0, rtx op1, rtx sel);
> >> > >
> >> > > Index: gcc/config/arm/arm.md
> >> > >
> >> >
> >>
> ==========================================================
> >> > =========
> >> > > --- gcc/config/arm/arm.md (revision 209852)
> >> > > +++ gcc/config/arm/arm.md (working copy)
> >> > > @@ -7555,6 +7555,20 @@
> >> > >  })
> >> > >
> >> > >
> >> > > +(define_expand "setmemsi"
> >> > > +  [(match_operand:BLK 0 "general_operand" "")
> >> > > +   (match_operand:SI 1 "const_int_operand" "")
> >> > > +   (match_operand:SI 2 "const_int_operand" "")
> >> > > +   (match_operand:SI 3 "const_int_operand" "")]
> >> > > +  "TARGET_32BIT"
> >> > > +{
> >> > > +  if (arm_gen_setmem (operands))
> >> > > +    DONE;
> >> > > +
> >> > > +  FAIL;
> >> > > +})
> >> > > +
> >> > > +
> >> > >  ;; Move a block of memory if it is word aligned and MORE than 2
> >> > > words
> >> > long.
> >> > >  ;; We could let this apply for blocks of less than this, but it
> >> > > clobbers so  ;; many registers that there is then probably a
> >> > > better
> > way.
> >> > > Index: gcc/testsuite/gcc.target/arm/memset-inline-6.c
> >> > >
> >> >
> >>
> ==========================================================
> >> > =========
> >> > > --- gcc/testsuite/gcc.target/arm/memset-inline-6.c        (revision 0)
> >> > > +++ gcc/testsuite/gcc.target/arm/memset-inline-6.c        (revision 0)
> >> >
> >> > Have you tested these when the compiler was configured with
> >> > "--with- cpu=cortex-a9"?
> >> Here is the tricky part.
> >> For compiler configured with "--with-tune=cortex-a9", the neon
> >> related cases
> >> (4/5/6/8/9) would fail because we have no way to determine that we
> >> are compiling with cortex-a9 tune here.
> 
> There is no easy way of fixing this.
> 
> >> For compiler configured with "--with-cpu=cortex-a9", the test cases
> >> would pass but I think this is a mistake.  It reveals an issue that
> >> GCC won't
> > pass "-
> >> mcpu=cortex-a9" to cc1, resulting in cortex-a8 tune is selected.  It
> >> just
> > makes
> >> no sense.
> >> With these issues, I didn't change the tests for now.
> 
> I think we'll just have to take the hit on noise in other configurations.
> 
> > Precisely, I configured gcc with options "--with-arch=armv7-a
> > --with-cpu|--with-tune=cortex-a9".
> > I read gcc documents and realized that "-mcpu" is ignored when
> > "-march" is specified.  I don't know why gcc acts in this manner, but
> > it leads to inconsistent configuration/command line behavior.
> > If we configure GCC with "--with-arch=armv7-a --with-cpu=cortex-a9",
> > then only "-march=armv7-a" is passed to cc1.
> 
> That kind of configuration is warned today but no one pays attention to that.
> See James's proposal to promote this to an error.
> 
> > If we compile with "-march=armv7-a -mcpu=cortex-a9", then gcc works
> > fine and passes "-march=armv7-a -mcpu=cortex-a9" to cc1.
> 
> That should be fine.
> 
> >
> > Even more weird cc1 warns that "switch -mcpu=cortex-m4 conflicts with
> > -march=armv7-m switch".
> 
> 
> This is OK unless there are objections in the next 24 hours.
> 
> Please watch out for any fallout - Certainly rebase and retest before applying
> and please post the rebased version for archival purposes.
> 

Hi Ramana,
This is the rebased patch, there is no conflict against latest trunk.  I am still doing some tests.  Is it OK if tests are fine?
Also, it depends on patch at https://gcc.gnu.org/ml/gcc-patches/2014-04/msg01923.html, I will update that patch two.

Thanks,
bin