[PATCH][ARM][gas] Warn on deprecated ldm r<n>, {..., sp,...} and ldm r<n>, {...,lr,pc,...} forms
Kyrill Tkachov
kyrylo.tkachov@arm.com
Thu Feb 12 15:57:00 GMT 2015
Ping.
https://sourceware.org/ml/binutils/2015-02/msg00029.html
Richard, if we apply this patch we'll get some noise in the GCC 5.0
testsuite since some of the tests there generate the sequences with
-mapcs (which is deprecated for GCC 5).
I have a patch to adjust the testcases here:
https://gcc.gnu.org/ml/gcc-patches/2015-01/msg01014.html
but I'm not sure if it will be accepted.
Do we want to go ahead with this change?
Thanks,
Kyrill
On 04/02/15 11:52, Kyrill Tkachov wrote:
> On 20/01/15 14:00, Richard Earnshaw wrote:
>> On 20/01/15 10:13, Kyrill Tkachov wrote:
>>> On 20/01/15 10:10, Kyrill Tkachov wrote:
>>>> 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.
>>>> Shall we go the way of as_tsktsk then and convert the other deprecation
>>>> warnings to as_tsktsk.
>>> That should have been a question :)
>>>
>> Yes, we should be consistent. And I think tsktsk is the right level for
>> these. We really do understand what the user has asked for, it's just
>> that it has *potentially* undefined behaviour at run time.
>>
>> A separate patch to make the messages consistent would be welcomed. For
>> this patch, just fix it to use tsktsk.
> Ok, here it is, using as_tsktsk.
> I'll look into moving the other deprecation warnings to tsktsk as well,
> though I guess the -mwarn-deprecated option will be not "technically"
> warning the user, just notifying?
>
> Kyrill
>
> [gas/]
> 2015-02-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>
> * config/tc-arm.c (encode_ldmstm): Add deprecation message when
> SP or both LR and PC are used in ldm register list.
>
> [gas/testsuite]
> 2015-02-04 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.
>
>
>
>> R.
>>
>>> Kyrill
>>>
>>>> I do see some deprecations using as_tsktsk and some using
>>>> as_warn.
>>>>
>>>> Kyrill
>>>>
>>>>> 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.
>>>>>
>>>>> 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
>>>>>>>
>>> >
More information about the Binutils
mailing list