Re: [Patch, ARM][2/8] Epilogue in RTL: new patterns for int regs

[Richard Earnshaw <rearnsha at arm dot com>]

On 31/05/12 14:53, Greta Yorsh wrote:
> This patch adds new define_insn patterns for epilogue with integer
> registers.
> 
> The patterns can handle pop multiple with writeback and return (loading into
> PC directly).
> To handle return, the patterns use a new special predicate
> pop_multiple_return, that uses ldm_stm_operation_p function from a previous
> patch. To output assembly, the patterns use a new function
> arm_output_multireg_pop.
> 
> This patch also adds a new function arm_emit_multi_reg_pop
> that emits RTL that matches the new pop patterns for integer registers.
> This is a helper function for epilogue expansion. It is used by a later
> patch.
> 
> ChangeLog:
> 
> gcc
> 
> 2012-05-31  Ian Bolton  <ian.bolton@arm.com>
>             Sameera Deshpande  <sameera.deshpande@arm.com>
>             Greta Yorsh  <greta.yorsh@arm.com>
> 
>         * config/arm/arm.md (load_multiple_with_writeback) New define_insn.
>         (load_multiple, pop_multiple_with_writeback_and_return) Likewise.
>         (pop_multiple_with_return, ldr_with_return) Likewise.
>         * config/arm/predicates.md (pop_multiple_return) New special
> predicate.
>         * config/arm/arm-protos.h (arm_output_multireg_pop) New declaration.
>         * config/arm/arm.c (arm_output_multireg_pop) New function.
>         (arm_emit_multi_reg_pop): New function.
>         (ldm_stm_operation_p): Check SP in the register list.
> 
> 
> 2-patterns.patch.txt
> 
> 


> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 4717725..9093801 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -13815,6 +13815,84 @@ vfp_output_fldmd (FILE * stream, unsigned int base, int reg, int count)
>  }
>  
>  
> +/* OPERANDS[0] is the entire list of insns that constitute pop,
> +   OPERANDS[1] is the base register, RETURN_PC is true iff return insn
> +   is in the list, UPDATE is true iff the list contains explicit
> +   update of base register.
> + */

Close of comment should not be on a separate line.

> +void
> +arm_output_multireg_pop (rtx *operands, bool return_pc, rtx cond, bool reverse,
> +                         bool update)

> +  offset += return_pc ? 1 : 0;
> +
> +  /* Is the base register in the list? */

Two spaces at end of comment before */.

> +  for (i = offset; i < num_saves; i++)
> +    {
> +      regno = REGNO (XEXP (XVECEXP (operands[0], 0, i), 0));
> +      /* If SP is in the list, then the base register must be SP. */

And here.

> +      gcc_assert ((regno != SP_REGNUM) || (regno_base == SP_REGNUM));
> +      /* If base register is in the list, there must be no explicit update.  */
> +      if (regno == regno_base)
> +        gcc_assert (!update);
> +    }
> +
> +  conditional = reverse ? "%?%D0" : "%?%d0";
> +  if ((regno_base == SP_REGNUM) && TARGET_UNIFIED_ASM)
> +    {
> +      /* Output pop (not stmfd) because it has a shorter encoding. */

And here.

> +      gcc_assert (update);
> +      sprintf (pattern, "pop%s\t{", conditional);
> +    }
> +  else
> +    {
> +      /* Output ldmfd when the base register is SP, otherwise output ldmia.
> +         It's just a convention, their semantics are identical.  */
> +      if (regno_base == SP_REGNUM)
> +        sprintf (pattern, "ldm%sfd\t", conditional);
> +      else if (TARGET_UNIFIED_ASM)
> +        sprintf (pattern, "ldmia%s\t", conditional);
> +      else
> +        sprintf (pattern, "ldm%sia\t", conditional);
> +
> +      strcat (pattern, reg_names[regno_base]);
> +      if (update)
> +        strcat (pattern, "!, {");
> +      else
> +        strcat (pattern, ", {");
> +    }
> +
> +  /* Output the first destination register. */

And here.

> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> index ed33c9b..862ccf4 100644
> --- a/gcc/config/arm/arm.md
> +++ b/gcc/config/arm/arm.md
> @@ -10959,6 +10959,89 @@
>    [(set_attr "type" "f_fpa_store")]
>  )
>  
> +;; Pop (as used in epilogue RTL)
> +;;
> +(define_insn "*load_multiple_with_writeback"
> +  [(match_parallel 0 "load_multiple_operation"
> +    [(set (match_operand:SI 1 "s_register_operand" "+rk")
> +          (plus:SI (match_dup 1)
> +                   (match_operand:SI 2 "const_int_operand" "I")))
> +     (set (match_operand:SI 3 "s_register_operand" "=rk")
> +          (mem:SI (match_dup 1)))
> +        ])]
> +  "TARGET_32BIT && (reload_in_progress || reload_completed)"
> +  "*
> +  {
> +    arm_output_multireg_pop (operands, /*return_pc=*/FALSE,
> +                                       /*cond=*/const_true_rtx,
> +                                       /*reverse=*/FALSE,
> +                                       /*update=*/TRUE);

Use lower case for TRUE and FALSE.  Several instances later on as well.

OK with those changes.

R.