public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Thomas Preudhomme <thomas.preudhomme@foss.arm.com>
To: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>,
	Kyrill Tkachov <kyrylo.tkachov@arm.com>,
	Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>,
	"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH, GCC/ARM] Remove ARMv8-M code for D17-D31
Date: Thu, 29 Jun 2017 08:44:00 -0000	[thread overview]
Message-ID: <d284719e-a170-5d48-1b2f-b33121c9508b@foss.arm.com> (raw)
In-Reply-To: <faba24a3-e07d-403d-b3ba-cc725fcdc742@arm.com>

Hi Richard,

On 28/06/17 16:56, Richard Earnshaw (lists) wrote:
> On 20/06/17 16:01, Thomas Preudhomme wrote:
>> Hi,
>>
>> 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
>>
> 
> This is silently baking in dangerous assumptions about GCC's internal
> numbering of the registers.  That's not a good idea from a long-term
> portability perspective.
> 
> At the very least you need to assert that all the interesting registers
> are numbered in the range 0..63; but ideally the code should just handle
> pretty much any assignment of internal register numbers.

Well there is already this:

gcc_assert ((unsigned) maxregno <= sizeof (to_clear_mask) * __CHAR_BIT__);

> 
> Did you consider using sbitmaps rather than doing all the multi-word
> stuff by steam?

No but am happy to. I'll respin the patch.

Best regards,

Thomas

  reply	other threads:[~2017-06-29  8:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-20 15:01 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 [this message]
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

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=d284719e-a170-5d48-1b2f-b33121c9508b@foss.arm.com \
    --to=thomas.preudhomme@foss.arm.com \
    --cc=Richard.Earnshaw@arm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kyrylo.tkachov@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).