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

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

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