public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
To: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>,
	"Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH, GCC/ARM, ping] Remove ARMv8-M code for D17-D31
Date: Thu, 28 Sep 2017 16:13:00 -0000	[thread overview]
Message-ID: <49cae04c-8969-e720-d144-be07e1ba61b5@foss.arm.com> (raw)
In-Reply-To: <59AFADD3.10303@foss.arm.com>

Committed (sorry for delay).

Best regards,

Thomas

On 06/09/17 09:12, Kyrill Tkachov wrote:
> 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
> 

      reply	other threads:[~2017-09-28 16:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-20 15:01 [PATCH, GCC/ARM] " Thomas Preudhomme
2017-06-28 15:12 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-06-28 15:57 ` [PATCH, GCC/ARM] " Richard Earnshaw (lists)
2017-06-29  8:44   ` Thomas Preudhomme
2017-07-06 12:36   ` Thomas Preudhomme
2017-07-07 14:19     ` Richard Earnshaw (lists)
2017-07-12  8:59       ` Thomas Preudhomme
2017-07-17  8:52         ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-07-17 16:25           ` Thomas Preudhomme
2017-08-23 11:01             ` Thomas Preudhomme
2017-08-25 13:23               ` [PATCH, GCC/ARM] " Thomas Preudhomme
2017-09-05  9:04                 ` [PATCH, GCC/ARM, ping] " Thomas Preudhomme
2017-09-06  8:12                   ` Kyrill Tkachov
2017-09-28 16:13                     ` Thomas Preudhomme [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49cae04c-8969-e720-d144-be07e1ba61b5@foss.arm.com \
    --to=thomas.preudhomme@foss.arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kyrylo.tkachov@foss.arm.com \
    --cc=ramana.radhakrishnan@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).