Re: [PATCH][ARM][gas] Warn on deprecated ldm r<n>, {..., sp,...} and ldm r<n>, {...,lr,pc,...} forms

[Richard Earnshaw <rearnsha at arm dot com>]

On 14/01/15 11:32, Ramana Radhakrishnan wrote:
> On Wed, Jan 14, 2015 at 11:12 AM, Richard Earnshaw <rearnsha@arm.com> wrote:
>> On 14/01/15 10:24, Kyrill Tkachov wrote:
>>> Hi all,
>>>
>>> ARMv7-A deprecates ldm instructions that use the SP register in their list
>>> as well as ldm instructions that use both LR and PC. GAS should warn
>>> about them.
>>>
>>> This patch adds a warning for these forms.
>>> Tests are added/updated.
>>>
>>> Tested check-gas in arm-none-eabi and I've had it sitting in my tree
>>> being tested
>>> with gcc for a few months now.
>>>
>>> What do people think?
>>>
>>
>> I think firstly, that we should use as_tsktsk for deprecations, rather
>> than warnings.
> 
> as_tsktsk would be better but we need to change a whole load of
> deprecations to this form rather than warnings in the ARM port. I
> don't think all our other deprecations use tsktsk. I think we need to
> move people on when they use this on v7-a and newer revisions of the
> architecture.
> 
>  I don't like hiding these behind flags or folks will not fix their
> issues without the compiler or assembler warning / shouting at them.
> 
>>
>> I'm still somewhat concerned about the use of SP in LDM lists.  This is
>> going to make any legacy ABI code generated by GCC (-mapcs-frame) very
>> verbose.  I don't expect we will want to fix GCC to avoid this sequence.
> 
> 
> Personally I think (not very strongly) we should fix GCC to avoid this
> sequence if we can and then deprecate all uses of mapcs-frame.
> Unfortunately it's one of these options that appears to linger on in
> build systems for no apparently good reason.
> 

Avoiding the deprecated instruction while remaining compatible with the
APCS frame layout will have a noticable impact on performance, I
suspect.  Particularly on some older cores.

R.

> Ramana
> 
>>
>> I'd like opinions from others on this one...
>>
>> R.
>>
>>> Thanks,
>>> Kyrill
>>>
>>> [gas/]
>>> 2015-01-14  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      * config/tc-arm.c (encode_ldmstm): Add deprecation warning when
>>>      SP or both LR and PC are used in ldm register list.
>>>
>>> [gas/testsuite]
>>> 2015-01-14  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>>
>>>      * gas/cfi/cfi-arm-1.s: Change usage of sp to ip in 'ldmea'.
>>>      * gas/arm/arm-ldm-bad.s: New file.
>>>      * gas/arm/arm-ldm-bad.d: Likewise.
>>>      * gas/arm/arm-ldm-bad.l: Likewise.
>>>
>>>
>>> arm-gas-ldm-warnings.patch
>>>
>>>
>>> commit 945c748ad5075d3d0e7d54e6258d8ab9bcf5fa36
>>> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>>> Date:   Mon Jun 30 17:28:56 2014 +0100
>>>
>>>     [ARM][gas] ldm sp warnings
>>>
>>> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
>>> index ce0532b..5755b4b 100644
>>> --- a/gas/config/tc-arm.c
>>> +++ b/gas/config/tc-arm.c
>>> @@ -8130,6 +8130,15 @@ encode_ldmstm(int from_push_pop_mnem)
>>>       }
>>>      }
>>>
>>> +  if (inst.instruction & LOAD_BIT
>>> +      && ARM_CPU_HAS_FEATURE (cpu_variant, arm_ext_v7a))
>>> +    {
>>> +      if (range & (1 << REG_SP))
>>> +        as_warn (_("usage of SP in register list is deprecated"));
>>> +      else if (range & (1 << REG_PC) && range & (1 << REG_LR))
>>> +        as_warn (_("usage of both LR and PC in register list is deprecated"));
>>> +    }
>>> +
>>>    /* If PUSH/POP has only one register, then use the A2 encoding.  */
>>>    one_reg = only_one_reg_in_list (range);
>>>    if (from_push_pop_mnem && one_reg >= 0)
>>> diff --git a/gas/testsuite/gas/arm/arm-ldm-bad.d b/gas/testsuite/gas/arm/arm-ldm-bad.d
>>> new file mode 100644
>>> index 0000000..8ce6a54
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/arm/arm-ldm-bad.d
>>> @@ -0,0 +1,3 @@
>>> +#name: ARM ldm/stm warnings
>>> +#as: -march=armv7-a
>>> +#error-output: arm-ldm-bad.l
>>> diff --git a/gas/testsuite/gas/arm/arm-ldm-bad.l b/gas/testsuite/gas/arm/arm-ldm-bad.l
>>> new file mode 100644
>>> index 0000000..1609594
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/arm/arm-ldm-bad.l
>>> @@ -0,0 +1,5 @@
>>> +[^:]*: Assembler messages:
>>> +[^:]*:4: Warning: usage of both LR and PC in register list is deprecated
>>> +[^:]*:5: Warning: usage of both LR and PC in register list is deprecated
>>> +[^:]*:6: Warning: usage of SP in register list is deprecated
>>> +[^:]*:7: Warning: usage of SP in register list is deprecated
>>> diff --git a/gas/testsuite/gas/arm/arm-ldm-bad.s b/gas/testsuite/gas/arm/arm-ldm-bad.s
>>> new file mode 100644
>>> index 0000000..d16cf5d
>>> --- /dev/null
>>> +++ b/gas/testsuite/gas/arm/arm-ldm-bad.s
>>> @@ -0,0 +1,7 @@
>>> +     .global entry
>>> +     .text
>>> +entry:
>>> +     ldm sp, {r4-r7,r11,lr,pc}
>>> +     ldm sp!, {r4-r7,r11,lr,pc}
>>> +     ldm r1, {r4-r7,sp}
>>> +     ldm r1!, {r4-r7,sp}
>>> diff --git a/gas/testsuite/gas/cfi/cfi-arm-1.s b/gas/testsuite/gas/cfi/cfi-arm-1.s
>>> index d962442..481f4f2 100644
>>> --- a/gas/testsuite/gas/cfi/cfi-arm-1.s
>>> +++ b/gas/testsuite/gas/cfi/cfi-arm-1.s
>>> @@ -23,7 +23,7 @@ foo:
>>>       .cfi_offset r1,  -16
>>>       .cfi_offset s1,  -20
>>>       .cfi_offset d11, -48
>>> -
>>> -     ldmea   fp, {fp, sp, pc}
>>> +
>>> +     ldmea   fp, {fp, ip, pc}
>>>       .cfi_endproc
>>>       .size   foo, .-foo
>>>
>>
>>
>