From: Richard Sandiford <richard.sandiford@arm.com>
To: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH v2 5/7] aarch64,arm: Remove accepted_branch_protection_string
Date: Sun, 26 Nov 2023 14:50:50 +0000 [thread overview]
Message-ID: <mpt34wszhyt.fsf@arm.com> (raw)
In-Reply-To: <8ebe0679d0d92c8c39bffc98e1f30e2b29770e00.1699025214.git.szabolcs.nagy@arm.com> (Szabolcs Nagy's message of "Fri, 3 Nov 2023 15:36:32 +0000")
Szabolcs Nagy <szabolcs.nagy@arm.com> writes:
> On aarch64 this caused ICE with pragma push_options since
>
> commit ae54c1b09963779c5c3914782324ff48af32e2f1
> Author: Wilco Dijkstra <wilco.dijkstra@arm.com>
> CommitDate: 2022-06-01 18:13:57 +0100
>
> AArch64: Cleanup option processing code
>
> The failure is at pop_options:
>
> internal compiler error: ‘global_options’ are modified in local context
>
> On arm the variable was unused.
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.cc (aarch64_override_options_after_change_1):
> Do not override branch_protection options.
> (aarch64_override_options): Remove accepted_branch_protection_string.
> * config/arm/aarch-common.cc (BRANCH_PROTECT_STR_MAX): Remove.
> (aarch_parse_branch_protection): Remove
> accepted_branch_protection_string.
> * config/arm/arm.cc: Likewise.
OK. Not sure I fully understand the subtlety of how things worked
before Wilco's patch, but I agree it looks like all the results of
the parsing are correctly captured by Target options or TargetVariables
in aarch64.opt, and so no special handling is needed.
Thanks,
Richard
> ---
> unchanged from v1
> ---
> gcc/config/aarch64/aarch64.cc | 10 +---------
> gcc/config/arm/aarch-common.cc | 16 ----------------
> gcc/config/arm/arm.cc | 2 --
> 3 files changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 88594bed8ce..f8e8fefc8d8 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -323,8 +323,6 @@ bool aarch64_pcrelative_literal_loads;
> /* Global flag for whether frame pointer is enabled. */
> bool aarch64_use_frame_pointer;
>
> -char *accepted_branch_protection_string = NULL;
> -
> /* Support for command line parsing of boolean flags in the tuning
> structures. */
> struct aarch64_flag_desc
> @@ -18101,12 +18099,6 @@ aarch64_adjust_generic_arch_tuning (struct tune_params ¤t_tune)
> static void
> aarch64_override_options_after_change_1 (struct gcc_options *opts)
> {
> - if (accepted_branch_protection_string)
> - {
> - opts->x_aarch64_branch_protection_string
> - = xstrdup (accepted_branch_protection_string);
> - }
> -
> /* PR 70044: We have to be careful about being called multiple times for the
> same function. This means all changes should be repeatable. */
>
> @@ -18715,7 +18707,7 @@ aarch64_override_options (void)
> /* Return address signing is currently not supported for ILP32 targets. For
> LP64 targets use the configured option in the absence of a command-line
> option for -mbranch-protection. */
> - if (!TARGET_ILP32 && accepted_branch_protection_string == NULL)
> + if (!TARGET_ILP32 && aarch64_branch_protection_string == NULL)
> {
> #ifdef TARGET_ENABLE_PAC_RET
> aarch_ra_sign_scope = AARCH_FUNCTION_NON_LEAF;
> diff --git a/gcc/config/arm/aarch-common.cc b/gcc/config/arm/aarch-common.cc
> index 5b96ff4c2e8..cbc7f68a8bf 100644
> --- a/gcc/config/arm/aarch-common.cc
> +++ b/gcc/config/arm/aarch-common.cc
> @@ -659,9 +659,6 @@ arm_md_asm_adjust (vec<rtx> &outputs, vec<rtx> & /*inputs*/,
> return saw_asm_flag ? seq : NULL;
> }
>
> -#define BRANCH_PROTECT_STR_MAX 255
> -extern char *accepted_branch_protection_string;
> -
> static enum aarch_parse_opt_result
> aarch_handle_no_branch_protection (char* str, char* rest)
> {
> @@ -812,19 +809,6 @@ aarch_parse_branch_protection (const char *const_str, char** last_str)
> else
> *last_str = NULL;
> }
> -
> - if (res == AARCH_PARSE_OK)
> - {
> - /* If needed, alloc the accepted string then copy in const_str.
> - Used by override_option_after_change_1. */
> - if (!accepted_branch_protection_string)
> - accepted_branch_protection_string
> - = (char *) xmalloc (BRANCH_PROTECT_STR_MAX + 1);
> - strncpy (accepted_branch_protection_string, const_str,
> - BRANCH_PROTECT_STR_MAX + 1);
> - /* Forcibly null-terminate. */
> - accepted_branch_protection_string[BRANCH_PROTECT_STR_MAX] = '\0';
> - }
> return res;
> }
>
> diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
> index 6e933c80183..f49312cace0 100644
> --- a/gcc/config/arm/arm.cc
> +++ b/gcc/config/arm/arm.cc
> @@ -2424,8 +2424,6 @@ const struct tune_params arm_fa726te_tune =
> tune_params::SCHED_AUTOPREF_OFF
> };
>
> -char *accepted_branch_protection_string = NULL;
> -
> /* Auto-generated CPU, FPU and architecture tables. */
> #include "arm-cpu-data.h"
next prev parent reply other threads:[~2023-11-26 14:50 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-03 15:36 [PATCH v2 0/7] aarch64 GCS preliminary patches Szabolcs Nagy
2023-11-03 15:36 ` [PATCH v2 1/7] aarch64: Use br instead of ret for eh_return Szabolcs Nagy
2023-11-13 0:27 ` Hans-Peter Nilsson
2023-11-13 11:18 ` Szabolcs Nagy
2023-11-26 12:11 ` Richard Sandiford
2023-11-03 15:36 ` [PATCH v2 2/7] aarch64: Do not force a stack frame for EH returns Szabolcs Nagy
2023-11-26 12:12 ` Richard Sandiford
2023-11-03 15:36 ` [PATCH v2 3/7] aarch64: Add eh_return compile tests Szabolcs Nagy
2023-11-26 14:37 ` Richard Sandiford
2023-11-27 10:04 ` Szabolcs Nagy
2023-11-27 15:57 ` Szabolcs Nagy
2023-12-02 19:23 ` Andrew Pinski
2023-11-03 15:36 ` [PATCH v2 4/7] aarch64: Disable branch-protection for pcs tests Szabolcs Nagy
2023-11-03 15:36 ` [PATCH v2 5/7] aarch64,arm: Remove accepted_branch_protection_string Szabolcs Nagy
2023-11-26 14:50 ` Richard Sandiford [this message]
2023-11-03 15:36 ` [PATCH v2 6/7] aarch64,arm: Fix branch-protection= parsing Szabolcs Nagy
2023-12-07 13:02 ` Richard Earnshaw
2023-11-03 15:36 ` [PATCH v2 7/7] aarch64,arm: Move branch-protection data to targets Szabolcs Nagy
2023-12-07 13:13 ` Richard Earnshaw
2024-01-11 14:43 ` Szabolcs Nagy
2024-01-11 14:49 ` Richard Earnshaw (lists)
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=mpt34wszhyt.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=szabolcs.nagy@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).