Re: [PATCH, GCC/ARM, ping] Remove ARMv8-M code for D17-D31

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

Hi Thomas,

On 05/09/17 10:04, Thomas Preudhomme wrote:
> Ping?
>

This is ok if a bootstrap and test run on arm-none-linux-gnueabihf shows 
no problems.
Thanks,
Kyrill

> Best regards,
>
> Thomas
>
> On 25/08/17 12:18, Thomas Preudhomme wrote:
>> Hi,
>>
>> I've now also added a couple more changes:
>>
>> * size to_clear_bitmap according to maxregno to be consistent with 
>> its use
>> * use directly TARGET_HARD_FLOAT instead of clear_vfpregs
>>
>>
>> Original message below (ChangeLog unchanged):
>>
>> Function cmse_nonsecure_entry_clear_before_return has code to deal with
>> high VFP register (D16-D31) while ARMv8-M Baseline and Mainline both do
>> not support more than 16 double VFP registers (D0-D15). This makes this
>> security-sensitive code harder to read for not much benefit since
>> libcall for cmse_nonsecure_call functions do not deal with those high
>> VFP registers anyway.
>>
>> This commit gets rid of this code for simplicity and fixes 2 issues in
>> the same function:
>>
>> - stop the first loop when reaching maxregno to avoid dealing with VFP
>>    registers if targetting Thumb-1 or using -mfloat-abi=soft
>> - include maxregno in that loop
>>
>> ChangeLog entry is as follows:
>>
>> *** gcc/ChangeLog ***
>>
>> 2017-06-13  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>
>>      * config/arm/arm.c (arm_option_override): Forbid ARMv8-M Security
>>      Extensions with more than 16 double VFP registers.
>>      (cmse_nonsecure_entry_clear_before_return): Remove second entry of
>>      to_clear_mask and all code related to it.  Replace the remaining
>>      entry by a sbitmap and adapt code accordingly.
>>
>> Testing: Testsuite shows no regression when run for ARMv8-M Baseline and
>> ARMv8-M Mainline.
>>
>> Is this ok for trunk?
>>
>> Best regards,
>>
>> Thomas
>>
>> On 23/08/17 11:56, Thomas Preudhomme wrote:
>>> Ping?
>>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>> On 17/07/17 17:25, Thomas Preudhomme wrote:
>>>> My bad, found an off-by-one error in the sizing of bitmaps. Please 
>>>> find fixed patch in attachment.
>>>>
>>>> ChangeLog entry is unchanged:
>>>>
>>>> *** gcc/ChangeLog ***
>>>>
>>>> 2017-06-13  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>
>>>>      * config/arm/arm.c (arm_option_override): Forbid ARMv8-M Security
>>>>      Extensions with more than 16 double VFP registers.
>>>>      (cmse_nonsecure_entry_clear_before_return): Remove second 
>>>> entry of
>>>>      to_clear_mask and all code related to it.  Replace the remaining
>>>>      entry by a sbitmap and adapt code accordingly.
>>>>
>>>> Best regards,
>>>>
>>>> Thomas
>>>>
>>>> On 17/07/17 09:52, Thomas Preudhomme wrote:
>>>>> Ping?
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Thomas
>>>>>
>>>>> On 12/07/17 09:59, Thomas Preudhomme wrote:
>>>>>> Hi Richard,
>>>>>>
>>>>>> On 07/07/17 15:19, Richard Earnshaw (lists) wrote:
>>>>>>>
>>>>>>> Hmm, I think that's because really this is a partial 
>>>>>>> conversion.  It
>>>>>>> looks like doing this properly would involve moving that 
>>>>>>> existing code
>>>>>>> to use sbitmaps as well.  I think doing that would be better for
>>>>>>> long-term maintenance perspectives, but I'm not going to insist 
>>>>>>> that you
>>>>>>> do it now.
>>>>>>
>>>>>> There's also the assert later but I've found a way to improve it 
>>>>>> slightly. While switching to auto_sbitmap I also changed the code 
>>>>>> slightly to allocate directly bitmaps to the right size. Since 
>>>>>> the change is probably bigger than what you had in mind I'd 
>>>>>> appreciate if you can give me an OK again. See updated patch in 
>>>>>> attachment. ChangeLog entry is unchanged:
>>>>>>
>>>>>> 2017-06-13  Thomas Preud'homme <thomas.preudhomme@arm.com>
>>>>>>
>>>>>>      * config/arm/arm.c (arm_option_override): Forbid ARMv8-M 
>>>>>> Security
>>>>>>      Extensions with more than 16 double VFP registers.
>>>>>>      (cmse_nonsecure_entry_clear_before_return): Remove second 
>>>>>> entry of
>>>>>>      to_clear_mask and all code related to it.  Replace the 
>>>>>> remaining
>>>>>>      entry by a sbitmap and adapt code accordingly.
>>>>>>
>>>>>>>
>>>>>>> As a result I'll let you take the call as to whether you keep this
>>>>>>> version or go back to your earlier patch.  If you do decide to 
>>>>>>> keep this
>>>>>>> version, then see the comment below.
>>>>>>
>>>>>> Given the changes I'm more happy with how the patch looks now and 
>>>>>> making it go in can be a nice incentive to change other ARMv8-M 
>>>>>> Security Extension related code later on.
>>>>>>
>>>>>> Best regards,
>>>>>>
>>>>>> Thomas