Re: [PATCH 2/2] [ARM] Add support for -mpure-code in thumb-1 (v6m)

[Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>]

Hi Christophe,

On 12/17/19 3:31 PM, Kyrill Tkachov wrote:
>
> On 12/17/19 2:33 PM, Christophe Lyon wrote:
>> On Tue, 17 Dec 2019 at 11:34, Kyrill Tkachov
>> <kyrylo.tkachov@foss.arm.com> wrote:
>>> Hi Christophe,
>>>
>>> On 11/18/19 9:00 AM, Christophe Lyon wrote:
>>>> On Wed, 13 Nov 2019 at 15:46, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On Tue, 12 Nov 2019 at 12:13, Richard Earnshaw (lists)
>>>>> <Richard.Earnshaw@arm.com> wrote:
>>>>>> On 18/10/2019 14:18, Christophe Lyon wrote:
>>>>>>> +      bool not_supported = arm_arch_notm || flag_pic ||
>>>> TARGET_NEON;
>>>>>> This is a poor name in the context of the function as a whole.  
>>>>>> What's
>>>>>> not supported.  Please think of a better name so that I have some 
>>>>>> idea
>>>>>> what the intention is.
>>>>> That's to keep most of the code common when checking if -mpure-code
>>>>> and -mslow-flash-data are supported.
>>>>> These 3 cases are common to the two compilation flags, and
>>>>> -mslow-flash-data still needs to check TARGET_HAVE_MOVT in addition.
>>>>>
>>>>> Would "common_unsupported_modes" work better for you?
>>>>> Or I can duplicate the "arm_arch_notm || flag_pic || TARGET_NEON" in
>>>>> the two tests.
>>>>>
>>>> Hi,
>>>>
>>>> Here is an updated version, using "common_unsupported_modes" instead
>>>> of "not_supported", and fixing the typo reported by Kyrill.
>>>> The ChangeLog is still the same.
>>>>
>>>> OK?
>>>
>>> The name looks ok to me. Richard had a concern about Armv8-M Baseline,
>>> but I do see it being supported as you pointed out.
>>>
>>> So I believe all the concerns are addressed.
>> OK, thanks!
>>
>>> Thus the code is ok. However, please also updated the documentation for
>>> -mpure-code in invoke.texi (it currently states that a MOVT instruction
>>> is needed).
>>>
>> I didn't think about this :(
>> It currently says: "This option is only available when generating
>> non-pic code for M-profile targets with the MOVT instruction."
>>
>> I suggest to remove the "with the MOVT instruction" part. Is that OK
>> if I commit my patch and this doc change?
>
> Yes, I think that is simplest correct change to make.
>

Can you also send a patch to the changes.html page for GCC 10 making 
users aware that this restriction is now lifted?

Thanks,

Kyrill


> Thanks,
>
> Kyrill
>
>
>> Christophe
>>
>>> Thanks,
>>>
>>> Kyrill
>>>
>>>
>>>
>>>> Thanks,
>>>>
>>>> Christophe
>>>>
>>>>> Thanks,
>>>>>
>>>>> Christophe
>>>>>
>>>>>> R.