RE: [PATCH, ARM][1/4] New RTL patterns for LDRD/STRD in Thumb mode

["Greta Yorsh" <greta dot yorsh at arm dot com>]

On 18 October 2012 14:41, Richard Earnshaw wrote:
> > +/* Checks whether the operands are valid for use in an LDRD/STRD
instruction.
> > +   Assumes that RT, RT2, and RTN are REG.  This is guaranteed by the
patterns.
> > +   Assumes that the address in the base register RTN is word aligned.
Pattern
> > +   guarantees that both memory accesses use the same base register,
> > +   the offsets are constants within the range, and the gap between the
offsets is 4.
> > +   If preload complete then check that registers are legal.  WBACK
indicates whether
> > +   address is updated.  LOAD indicates whether memory access is load or
store.  */
> 
> ARM ARM terminology uses Rn for the base reg, so:
> 
> s/RTN/RN/

Fixed.

> 
> > +bool
> > +operands_ok_ldrd_strd (rtx rt, rtx rt2, rtx rtn, HOST_WIDE_INT offset,
> 
> s/rtn/rn/

Fixed.

> > +;; Patterns for LDRD/STRD in Thumb2 mode
> > +
> > +(define_insn "*thumb2_ldrd"
> > +  [(set (match_operand:SI 0 "s_register_operand" "=r")
> > +        (mem:SI (plus:SI (match_operand:SI 1 "s_register_operand" "rk")
> > +                         (match_operand:SI 2 "ldrd_strd_offset_operand"
"Do"))))
> > +   (set (match_operand:SI 3 "s_register_operand" "=r")
> > +        (mem:SI (plus:SI (match_dup 1)
> > +                         (match_operand:SI 4 "const_int_operand"
""))))]
> > +  "TARGET_LDRD && TARGET_THUMB2
> > +     && (current_tune->prefer_ldrd_strd &&
!optimize_function_for_size_p (cfun))
> 
> All these should be gated on "reload_completed" and not on the tune or 
> size optimization.

Removed the condition "!optimize_function_for_size_p (cfun))".

The condition "current_tune->prefer_ldrd_strd" is needed because the
patterns 
for LDRD/STRD appear before the patterns for LDM/STM that can match the same
RTL
(two register in the list). Condition "reload_completed" does not help with
it
because peephole optimizations in ldmstm.md may (after reload) create new
RTL insn 
that match this pattern.

> > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> > index f330da3..21d1aa8 100644
> > --- a/gcc/config/arm/arm.c
> > +++ b/gcc/config/arm/arm.c
> > @@ -12130,6 +12130,9 @@ offset_ok_for_ldrd_strd (HOST_WIDE_INT offset)
> >   {
> >     HOST_WIDE_INT max_offset;
> >
> > +  if (!TARGET_LDRD)
> > +    return false;
> > +
> 
> This seems to be in the wrong place.  If we don't have ldrd then the 
> question as to what is a valid offset is irrelevant.

Moved this condition to predicates.md and constraints.md.

Other uses of offset_ok_for_ldrd_strd are already guarded by the conditions.

I am attaching a new version of this patch. 

No regression on qemu for arm-none-eabi with cpu cortex-m4 and cortex-a15.

Ok for trunk?

Thank you,
Greta

ChangeLog


gcc/

2012-10-19  Sameera Deshpande  <sameera.deshpande@arm.com>
            Greta Yorsh  <Greta.Yorsh@arm.com>

        * config/arm/arm-protos.h (offset_ok_for_ldrd_strd): New
        declaration.
        (operands_ok_ldrd_strd): Likewise.
        * config/arm/arm.c (offset_ok_for_ldrd_strd): New function.
        (operands_ok_ldrd_strd): Likewise.
        * config/arm/arm.md (thumb2_ldrd, thumb2_ldrd_base): New patterns.
        (thumb2_ldrd_base_neg): Likewise.
        (thumb2_strd, thumb2_strd_base, thumb_strd_base_neg): Likewise.
        * predicates.md (ldrd_strd_offset_operand): New predicate.
        * config/arm/constraints.md (Do): New constraint.

Attachment: 1-thumb-patterns.patch.txt
Description: Text document