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

[Ramana Radhakrishnan]

On 8 April 2011 09:52, Carrot Wei <carrot@google.com> wrote:
> Sorry, which pattern in thumb2.md?

Sorry, I meant to say push_multi in arm.md . Not enough coffee yet this morning.

cheers
Ramana
>
> thanks
> Carrot
>
> On Fri, Apr 8, 2011 at 4:35 PM, Ramana Radhakrishnan
> <ramana.radhakrishnan@linaro.org> wrote:
>> 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
>>>>
>>>>
>>
>>
>