public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main.
@ 2022-10-31 15:36 Srinath Parvathaneni
  2022-12-06 11:31 ` Srinath Parvathaneni
  2022-12-06 17:58 ` Richard Earnshaw
  0 siblings, 2 replies; 3+ messages in thread
From: Srinath Parvathaneni @ 2022-10-31 15:36 UTC (permalink / raw)
  To: gcc-patches; +Cc: richard.earnshaw, kyrylo.tkachov

[-- Attachment #1: Type: text/plain, Size: 2868 bytes --]

Hi,

This patch adds the support for pacbti multlilib linking by making
"-mbranch-protection=none" as default in the command line for all M-profile
targets and uses "-mbranch-protection=none" for multilib matching. If any
valid value is passed to "-mbranch-protection" in the command line, this
new value overwrites the default value in the command line and uses
"-mbranch-protection=standard" for multilib matching.

Eg 1.

If the passed command line flags are:
a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto
b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto

After this patch the command line flags the compiler receives will be:
a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto -mbranch-protection=none
b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=none

"-mbranch-protection=none" will be used in the multilib matching.

Eg 2.

If the passed command line flags are:
a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret
b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret+bti

After this patch the command line flags the compiler receives will be:
a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret
b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret+bti

"-mbranch-protection=standard" will be used in the multilib matching.

Eg 3.

For A-profile target, if the passed command line flags are:
-march=armv8-a+simd -mfloat-abi=hard -mfpu=auto

Even after this patch the command line flags compiler receives will remain the same:
-march=armv8-a+simd -mfloat-abi=hard -mfpu=auto

Regression tested on arm-none-eabi and bootstrapped on arm-none-linux-gnueabihf.

Ok for master?

Regards,
Srinath.

gcc/ChangeLog:

2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

        * common/config/arm/arm-common.cc
        (arm_canon_branch_protection_option): Define new function.
        * config/arm/arm-cpus.in (armv8.1-m.main): Move dsp option below pacbti
        option.
        * config/arm/arm.h (arm_canon_branch_protection_option): Define function
        prototype.
        (CANON_BRANCH_PROTECTION_SPEC_FUNCTION): Define macro.
        (MBRANCH_PROTECTION_SPECS): Likewise.
        * config/arm/t-rmprofile (MULTI_ARCH_OPTS_RM): Add new options.
        (MULTI_ARCH_DIRS_RM): Add new directories.
        (MULTILIB_REQUIRED): Add new option.
        (MULTILIB_REUSE): Reuse existing multlibs.
        (MULTILIB_MATCHES): Match multilib strings.

gcc/testsuite/ChangeLog:

2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

        * gcc.target/arm/multilib.exp (multilib_config "rmprofile"): Update
        tests.
        * gcc.target/arm/pac-10.c: New test.
        * gcc.target/arm/pac-11.c: Likewise.
        * gcc.target/arm/pac-12.c: Likewise.

[-- Attachment #2: rb16143.patch.gz --]
[-- Type: application/gzip, Size: 7965 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main.
  2022-10-31 15:36 [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main Srinath Parvathaneni
@ 2022-12-06 11:31 ` Srinath Parvathaneni
  2022-12-06 17:58 ` Richard Earnshaw
  1 sibling, 0 replies; 3+ messages in thread
From: Srinath Parvathaneni @ 2022-12-06 11:31 UTC (permalink / raw)
  To: gcc-patches, Richard Earnshaw; +Cc: Srinath Parvathaneni

[-- Attachment #1: Type: text/plain, Size: 3366 bytes --]

Ping!!
________________________________
From: Gcc-patches <gcc-patches-bounces+srinath.parvathaneni=arm.com@gcc.gnu.org> on behalf of Srinath Parvathaneni via Gcc-patches <gcc-patches@gcc.gnu.org>
Sent: 31 October 2022 15:36
To: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>
Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>
Subject: [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main.

Hi,

This patch adds the support for pacbti multlilib linking by making
"-mbranch-protection=none" as default in the command line for all M-profile
targets and uses "-mbranch-protection=none" for multilib matching. If any
valid value is passed to "-mbranch-protection" in the command line, this
new value overwrites the default value in the command line and uses
"-mbranch-protection=standard" for multilib matching.

Eg 1.

If the passed command line flags are:
a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto
b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto

After this patch the command line flags the compiler receives will be:
a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto -mbranch-protection=none
b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=none

"-mbranch-protection=none" will be used in the multilib matching.

Eg 2.

If the passed command line flags are:
a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret
b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret+bti

After this patch the command line flags the compiler receives will be:
a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret
b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret+bti

"-mbranch-protection=standard" will be used in the multilib matching.

Eg 3.

For A-profile target, if the passed command line flags are:
-march=armv8-a+simd -mfloat-abi=hard -mfpu=auto

Even after this patch the command line flags compiler receives will remain the same:
-march=armv8-a+simd -mfloat-abi=hard -mfpu=auto

Regression tested on arm-none-eabi and bootstrapped on arm-none-linux-gnueabihf.

Ok for master?

Regards,
Srinath.

gcc/ChangeLog:

2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

        * common/config/arm/arm-common.cc
        (arm_canon_branch_protection_option): Define new function.
        * config/arm/arm-cpus.in (armv8.1-m.main): Move dsp option below pacbti
        option.
        * config/arm/arm.h (arm_canon_branch_protection_option): Define function
        prototype.
        (CANON_BRANCH_PROTECTION_SPEC_FUNCTION): Define macro.
        (MBRANCH_PROTECTION_SPECS): Likewise.
        * config/arm/t-rmprofile (MULTI_ARCH_OPTS_RM): Add new options.
        (MULTI_ARCH_DIRS_RM): Add new directories.
        (MULTILIB_REQUIRED): Add new option.
        (MULTILIB_REUSE): Reuse existing multlibs.
        (MULTILIB_MATCHES): Match multilib strings.

gcc/testsuite/ChangeLog:

2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>

        * gcc.target/arm/multilib.exp (multilib_config "rmprofile"): Update
        tests.
        * gcc.target/arm/pac-10.c: New test.
        * gcc.target/arm/pac-11.c: Likewise.
        * gcc.target/arm/pac-12.c: Likewise.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main.
  2022-10-31 15:36 [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main Srinath Parvathaneni
  2022-12-06 11:31 ` Srinath Parvathaneni
@ 2022-12-06 17:58 ` Richard Earnshaw
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Earnshaw @ 2022-12-06 17:58 UTC (permalink / raw)
  To: Srinath Parvathaneni, gcc-patches; +Cc: richard.earnshaw



On 31/10/2022 15:36, Srinath Parvathaneni via Gcc-patches wrote:
> Hi,
> 
> This patch adds the support for pacbti multlilib linking by making
> "-mbranch-protection=none" as default in the command line for all M-profile
> targets and uses "-mbranch-protection=none" for multilib matching. If any
> valid value is passed to "-mbranch-protection" in the command line, this
> new value overwrites the default value in the command line and uses
> "-mbranch-protection=standard" for multilib matching.
> 
> Eg 1.
> 
> If the passed command line flags are:
> a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto
> b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto
> 
> After this patch the command line flags the compiler receives will be:
> a) -march=armv8.1-m.main+mve -mfloat-abi=hard -mfpu=auto -mbranch-protection=none
> b) -mcpu=cortex-m85+nopacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=none
> 
> "-mbranch-protection=none" will be used in the multilib matching.
> 
> Eg 2.
> 
> If the passed command line flags are:
> a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret
> b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto  -mbranch-protection=pac-ret+bti
> 
> After this patch the command line flags the compiler receives will be:
> a) -march=armv8.1-m.main+mve+pacbti -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret
> b) -mcpu=cortex-m85 -mfloat-abi=hard -mfpu=auto -mbranch-protection=pac-ret+bti
> 
> "-mbranch-protection=standard" will be used in the multilib matching.
> 
> Eg 3.
> 
> For A-profile target, if the passed command line flags are:
> -march=armv8-a+simd -mfloat-abi=hard -mfpu=auto
> 
> Even after this patch the command line flags compiler receives will remain the same:
> -march=armv8-a+simd -mfloat-abi=hard -mfpu=auto
> 
> Regression tested on arm-none-eabi and bootstrapped on arm-none-linux-gnueabihf.
> 
> Ok for master?
> 
> Regards,
> Srinath.
> 
> gcc/ChangeLog:
> 
> 2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
> 
>          * common/config/arm/arm-common.cc
>          (arm_canon_branch_protection_option): Define new function.
>          * config/arm/arm-cpus.in (armv8.1-m.main): Move dsp option below pacbti
>          option.
>          * config/arm/arm.h (arm_canon_branch_protection_option): Define function
>          prototype.
>          (CANON_BRANCH_PROTECTION_SPEC_FUNCTION): Define macro.
>          (MBRANCH_PROTECTION_SPECS): Likewise.
>          * config/arm/t-rmprofile (MULTI_ARCH_OPTS_RM): Add new options.
>          (MULTI_ARCH_DIRS_RM): Add new directories.
>          (MULTILIB_REQUIRED): Add new option.
>          (MULTILIB_REUSE): Reuse existing multlibs.
>          (MULTILIB_MATCHES): Match multilib strings.
> 
> gcc/testsuite/ChangeLog:
> 
> 2022-10-28  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
> 
>          * gcc.target/arm/multilib.exp (multilib_config "rmprofile"): Update
>          tests.
>          * gcc.target/arm/pac-10.c: New test.
>          * gcc.target/arm/pac-11.c: Likewise.
>          * gcc.target/arm/pac-12.c: Likewise.

Your attachment this time is gzipped, which is almost as bad as 
octet-stream.  Please use text/plain attachments.

--- a/gcc/config/arm/arm-cpus.in
+++ b/gcc/config/arm/arm-cpus.in
@@ -746,8 +746,8 @@ begin arch armv8.1-m.main
   profile M
   isa ARMv8_1m_main
  # fp => FPv5-sp-d16; fp.dp => FPv5-d16
- option dsp add armv7em
   option pacbti add pacbti
+ option dsp add armv7em

Why is this needed?  It looks completely unnecessary.

+/* Automatically add -mbranch-protection=none for M-profile targets if
+   -mbranch-protection value isn't specified via the command line.  */
+#define MBRANCH_PROTECTION_SPECS				\
+  "%{!mbranch-protection=*:%:canon_branch_protection(%{march=*:arch %*;" \
+  "mcpu=*:cpu %*;:})}"
+

This doesn't canonicalize the branch-protection option, it provides a 
default if none was specified.  So if we really need this operation (see 
below) it should be renamed accordingly.

  MULTI_ARCH_OPTS_RM	+= mbranch-protection=standard
-MULTI_ARCH_DIRS_RM	+= mbranch-protection
+MULTI_ARCH_DIRS_RM	+= branch_protection_on
+MULTI_ARCH_OPTS_RM	+= mbranch-protection=none
+MULTI_ARCH_DIRS_RM	+= branch_protection_off

These options are related (you'll never need both), so it should be 
written as

  MULTI_ARCH_OPTS_RM	+= mbranch-protection=standard/mbranch-protection=none
and then add
MULTI_ARCH_DIRS_RM	+= mbranch_protection_on mbranch_protection_off

as a single line.  But I think it would be better to rename the 
directory names as "bp" and "bp_off".

Except that...

Why do we need a separate bp_off multilib at all? That's the default and 
the multilib framework can be told how to handle that ...

Firstly, create a new header, lets call it arm-mlib.h, containing

#define MULTILIB_DEFAULTS { "mbranch-protection=none" }

And then arrange to add this new header when the multilib framework is 
enabled via config.gcc.  You'll need to add this to ${tm_file} when we 
add the extra multilibs (search for "aprofile|rmprofile" in config.gcc).

Now you no longer need to handle this case at all in the multilib 
framework, since the compiler now knows that this is equivalent to 
omitting the option entirely.  That should eliminate a lot of the code 
you're adding, along with the canonicalization step.

Secondly, you appear to be missing support for the other variants of 
-mbranch-protection.  The manual says that we have
none, standard, pac-ret, standard+leaf and pac-ret+leaf and we need 
MULTILIB_MATCHES rules to reuse the standard variant for these cases.

Finally, please make sure this is tested and works as expected when you 
have both a-profile and rm-profile multilibs in the same build.

R.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-06 17:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-31 15:36 [GCC][PATCH v2] arm: Add pacbti related multilib support for armv8.1-m.main Srinath Parvathaneni
2022-12-06 11:31 ` Srinath Parvathaneni
2022-12-06 17:58 ` Richard Earnshaw

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).