On 31/05/16 15:15, Kyrill Tkachov wrote:
>
> +/* Compute the atrribute "length" of insn.  Currently, this function 
> is used
> +   for "*load_multiple_with_writeback", "*pop_multiple_with_return" and
> +   "*pop_multiple_with_writeback_and_return".  */
> +
>
> s/atrribute/attribute/
>
> Also, please add a description of the function arguments to the 
> function comment.
>
>  +int
> +arm_attr_length_pop_multi(rtx *operands, bool return_pc, bool 
> write_back_p)
> +{
>
> space between function name and '('.
>
>  +  /* ARM mode.  */
> +  if (TARGET_ARM)
> +    return 4;
> +  /* Thumb1 mode.  */
> +  if (TARGET_THUMB1)
> +    return 2;
> +
> +  /* For POP/PUSH/LDM/STM under Thumb2 mode, we can use 16-bit Thumb 
> encodings
> +     if the register list is 8-bit.  Normally this means all 
> registers in the
> +     list must be LO_REGS, that is (R0 -R7).  If any HI_REGS used, 
> then we must
> +     use 32-bit encodings.  But there are two exceptions to this rule 
> where
> +     special HI_REGS used in PUSH/POP.
> +
> +     1. 16-bit Thumb encoding POP can use an 8-bit register list, and an
> +        additional bit, P, that corresponds to the PC.  This means it 
> can access
> +    any of {PC, R7-R0}.
>
>
> It took me a bit to figure it out but this bit 'P' you're talking 
> about is a just a bit
> in the illustration in the ARM Architecture Reference Manual (section 
> A8.8.131).
> I don't think it's useful to refer to it.
> This would be better phrased as "The encoding is 16 bits wide if the 
> register list contains
> only the registers in {R0 - R7} or the PC".
>
>  +     2. 16-bit Thumb encoding PUSH can use an 8-bit register list, 
> and an
> +    additional bit, M , that corresponds to the LR.  This means it can
> +    access any of {LR, R7-R0}.  */
> +
>
> This function only deals with POP instructions, so talking about PUSH 
> encodings
> is confusing. I suggest you drop point 2).

Thanks, all fixed.

Meanwhile I splitted the comments to keep PUSH part in arm_attr_length_push_multi.

Patch updated.

OK for trunk?

gcc/

2016-06-02  Jiong Wang  <jiong.wang@arm.com>

     PR target/71061
     * config/arm/arm-protos.h (arm_attr_length_pop_multi): New declaration.
     * config/arm/arm.c (arm_attr_length_pop_multi): New function to return
     length for pop patterns.
     (arm_attr_length_push_multi): Update comments.
     * config/arm/arm.md (*load_multiple_with_writeback): Set "length" attribute.
     (*pop_multiple_with_writeback_and_return): Likewise.
     (*pop_multiple_with_return): Likewise.