Re: [PATCH, ARM] PR47855 Compute attr "length" for some thumb2 insns

[Ramana Radhakrishnan <ramana.radhakrishnan@linaro.org>]

On 08/04/11 08:36, Carrot Wei wrote:
> Hi
>
> This patch moves the length computation of push_multi into a C
> function to avoid the usage of GNU extension.

Please put the comment regarding the calculation of the length attribute 
along with the pattern in thumb2.md as well.

>
> Tested on qemu without regression. OK to install?

Ok with that change.

Thanks for fixing this up.

cheers
Ramana
>
> thanks
> Carrot
>
> ChangeLog:
> 2011-04-08  Wei Guozhi<carrot@google.com>
>
>          PR target/47855
>          * config/arm/arm-protos.h (arm_attr_length_push_multi): New prototype.
>          * config/arm/arm.c (arm_attr_length_push_multi): New function.
>          * config/arm/arm.md (*push_multi): Change the length computation to
>          call a C function.
>
>
> Index: arm.c
> ===================================================================
> --- arm.c	(revision 172158)
> +++ arm.c	(working copy)
> @@ -23696,4 +23696,30 @@ arm_preferred_rename_class (reg_class_t
>       return NO_REGS;
>   }
>
> +/* Compute the atrribute "length" of insn "*push_multi".
> +   So this function MUST be kept in sync with that insn pattern.  */
> +int
> +arm_attr_length_push_multi(rtx parallel_op, rtx first_op)
> +{
> +  int i, regno, hi_reg;
> +  int num_saves = XVECLEN (parallel_op, 0);
> +
> +  /* ARM mode.  */
> +  if (TARGET_ARM)
> +    return 4;
> +
> +  /* Thumb2 mode.  */
> +  regno = REGNO (first_op);
> +  hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)&&  (regno != LR_REGNUM);
> +  for (i = 1; i<  num_saves&&  !hi_reg; i++)
> +    {
> +      regno = REGNO (XEXP (XVECEXP (parallel_op, 0, i), 0));
> +      hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)&&  (regno != LR_REGNUM);
> +    }
> +
> +  if (!hi_reg)
> +    return 2;
> +  return 4;
> +}
> +
>   #include "gt-arm.h"
> Index: arm-protos.h
> ===================================================================
> --- arm-protos.h	(revision 172158)
> +++ arm-protos.h	(working copy)
> @@ -152,6 +152,7 @@ extern void arm_expand_sync (enum machin
>   extern const char *arm_output_memory_barrier (rtx *);
>   extern const char *arm_output_sync_insn (rtx, rtx *);
>   extern unsigned int arm_sync_loop_insns (rtx , rtx *);
> +extern int arm_attr_length_push_multi(rtx, rtx);
>
>   #if defined TREE_CODE
>   extern void arm_init_cumulative_args (CUMULATIVE_ARGS *, tree, rtx, tree);
> Index: arm.md
> ===================================================================
> --- arm.md	(revision 172158)
> +++ arm.md	(working copy)
> @@ -10290,27 +10290,7 @@
>     }"
>     [(set_attr "type" "store4")
>      (set (attr "length")
> -	(if_then_else
> -	   (and (ne (symbol_ref "TARGET_THUMB2") (const_int 0))
> -		(ne (symbol_ref "{
> -		    /* Check if there are any high register (except lr)
> -		       references in the list. KEEP the following iteration
> -		       in sync with the template above.  */
> -		    int i, regno, hi_reg;
> -		    int num_saves = XVECLEN (operands[2], 0);
> -		    regno = REGNO (operands[1]);
> -		    hi_reg = (REGNO_REG_CLASS (regno) == HI_REGS)
> -			&&  (regno != LR_REGNUM);
> -		    for (i = 1; i<  num_saves&&  !hi_reg; i++)
> -		      {
> -			regno = REGNO (XEXP (XVECEXP (operands[2], 0, i), 0));
> -			hi_reg |= (REGNO_REG_CLASS (regno) == HI_REGS)
> -				&&  (regno != LR_REGNUM);
> -		      }
> -		    !hi_reg;    }")
> -		  (const_int 0)))
> -	   (const_int 2)
> -	   (const_int 4)))]
> +	(symbol_ref "arm_attr_length_push_multi (operands[2], operands[1])"))]
>   )
>
>   (define_insn "stack_tie"
>
> On Thu, Apr 7, 2011 at 7:30 PM, Ramana Radhakrishnan
> <ramana.radhakrishnan@linaro.org>  wrote:
>> On 07/04/11 12:08, Carrot Wei wrote:
>>>
>>> On Thu, Apr 7, 2011 at 5:31 PM, Richard Sandiford
>>> <richard.sandiford@linaro.org>    wrote:
>>>>
>>>> Hi Carrot,
>>>>
>>>> Sorry if this has already been reported, but the patch breaks bootstrap
>>>> of arm-linux-gnueabi (or cross builds with --enable-werror).  The problem
>>>> is that this...
>>>>
>>>> uses a statement expression -- i.e. ({ code; result; }) -- which is
>>>> a GNU extension.
>>>>
>>>> I suppose a simple fix would be to put the code into an arm.c function.
>>>>
>>>
>>> Thank you for the report, I will add this fix to the second part of the
>>> patch.
>>
>> It would be worth unbreaking bootstrap as a separate patch and not
>> conflating it with a different set of improvements.
>>
>> Ramana
>>
>>>
>>> Carrot
>>
>>