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

[Carrot Wei <carrot at google dot com>]

Sorry, which pattern in thumb2.md?

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
>>>
>>>
>
>