On Fri, Feb 5, 2021 at 2:49 PM Martin Liška <mli...@suse.cz> wrote:
>
> Hello.
>
> As seen the flag -fpatchable-function-entry is properly marked as
> Optimization.
> However, it's the argument is parsed early and stored into the following
> tuple:
>
> ; How many NOP insns to place at each function entry by default
> Variable
> HOST_WIDE_INT function_entry_patch_area_size
>
> ; And how far the real asm entry point is into this area
> Variable
> HOST_WIDE_INT function_entry_patch_area_start
>
> That does not work with set_current_function where per-function arguments are
> restored.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
OK.
Thanks,
Richard.
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> PR lto/98971
> * cfgexpand.c (pass_expand::execute): Parse per-function option
> flag_patchable_function_entry and use it.
> * common.opt: Remove function_entry_patch_area_size and
> function_entry_patch_area_start global variables.
> * opts.c (parse_and_check_patch_area): New function.
> (common_handle_option): Use it.
> * opts.h (parse_and_check_patch_area): New function.
> * toplev.c (process_options): Parse and use
> function_entry_patch_area_size.
> ---
> gcc/cfgexpand.c | 6 +++--
> gcc/common.opt | 10 +-------
> gcc/opts.c | 65 +++++++++++++++++++++++++++++++------------------
> gcc/opts.h | 4 +++
> gcc/toplev.c | 6 ++++-
> 5 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 8d20ca6cefb..aef9e916fcd 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see
> #include "tree-ssa-address.h"
> #include "output.h"
> #include "builtins.h"
> +#include "opts.h"
>
> /* Some systems use __main in a way incompatible with its use in gcc, in
> these
> cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN
> to
> @@ -6845,8 +6846,9 @@ pass_expand::execute (function *fun)
> if (crtl->tail_call_emit)
> fixup_tail_calls ();
>
> - unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> - unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> + HOST_WIDE_INT patch_area_size, patch_area_entry;
> + parse_and_check_patch_area (flag_patchable_function_entry, false,
> + &patch_area_size, &patch_area_entry);
>
> tree patchable_function_entry_attr
> = lookup_attribute ("patchable_function_entry",
> diff --git a/gcc/common.opt b/gcc/common.opt
> index a8a2b67a99d..c75dd36843e 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -144,14 +144,6 @@ bool flag_stack_usage_info = false
> Variable
> int flag_debug_asm
>
> -; How many NOP insns to place at each function entry by default
> -Variable
> -HOST_WIDE_INT function_entry_patch_area_size
> -
> -; And how far the real asm entry point is into this area
> -Variable
> -HOST_WIDE_INT function_entry_patch_area_start
> -
> ; Balance between GNAT encodings and standard DWARF to emit.
> Variable
> enum dwarf_gnat_encodings gnat_encodings = DWARF_GNAT_ENCODINGS_DEFAULT
> @@ -2309,7 +2301,7 @@ Common Var(flag_profile_reorder_functions) Optimization
> Enable function reordering that improves code placement.
>
> fpatchable-function-entry=
> -Common Joined Optimization
> +Common Var(flag_patchable_function_entry) Joined Optimization
> Insert NOP instructions at each function entry.
>
> frandom-seed
> diff --git a/gcc/opts.c b/gcc/opts.c
> index 1f1cf8388f7..fc5f61e13cc 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -2130,6 +2130,44 @@ check_alignment_argument (location_t loc, const char
> *flag, const char *name,
> }
> }
>
> +/* Parse argument of -fpatchable-function-entry option ARG and store
> + corresponding values to PATCH_AREA_SIZE and PATCH_AREA_START.
> + If REPORT_ERROR is set to true, generate error for a problematic
> + option arguments. */
> +
> +void
> +parse_and_check_patch_area (const char *arg, bool report_error,
> + HOST_WIDE_INT *patch_area_size,
> + HOST_WIDE_INT *patch_area_start)
> +{
> + *patch_area_size = 0;
> + *patch_area_start = 0;
> +
> + if (arg == NULL)
> + return;
> +
> + char *patch_area_arg = xstrdup (arg);
> + char *comma = strchr (patch_area_arg, ',');
> + if (comma)
> + {
> + *comma = '\0';
> + *patch_area_size = integral_argument (patch_area_arg);
> + *patch_area_start = integral_argument (comma + 1);
> + }
> + else
> + *patch_area_size = integral_argument (patch_area_arg);
> +
> + if (*patch_area_size < 0
> + || *patch_area_size > USHRT_MAX
> + || *patch_area_start < 0
> + || *patch_area_start > USHRT_MAX
> + || *patch_area_size < *patch_area_start)
> + if (report_error)
> + error ("invalid arguments for %<-fpatchable-function-entry%>");
> +
> + free (patch_area_arg);
> +}
> +
> /* Print help when OPT__help_ is set. */
>
> void
> @@ -2671,30 +2709,9 @@ common_handle_option (struct gcc_options *opts,
>
> case OPT_fpatchable_function_entry_:
> {
> - char *patch_area_arg = xstrdup (arg);
> - char *comma = strchr (patch_area_arg, ',');
> - if (comma)
> - {
> - *comma = '\0';
> - function_entry_patch_area_size =
> - integral_argument (patch_area_arg);
> - function_entry_patch_area_start =
> - integral_argument (comma + 1);
> - }
> - else
> - {
> - function_entry_patch_area_size =
> - integral_argument (patch_area_arg);
> - function_entry_patch_area_start = 0;
> - }
> - if (function_entry_patch_area_size < 0
> - || function_entry_patch_area_size > USHRT_MAX
> - || function_entry_patch_area_start < 0
> - || function_entry_patch_area_start > USHRT_MAX
> - || function_entry_patch_area_size
> - < function_entry_patch_area_start)
> - error ("invalid arguments for %<-fpatchable-function-entry%>");
> - free (patch_area_arg);
> + HOST_WIDE_INT patch_area_size, patch_area_start;
> + parse_and_check_patch_area (arg, true, &patch_area_size,
> + &patch_area_start);
> }
> break;
>
> diff --git a/gcc/opts.h b/gcc/opts.h
> index a6ed2e3953e..bafc790112b 100644
> --- a/gcc/opts.h
> +++ b/gcc/opts.h
> @@ -475,6 +475,10 @@ extern bool parse_and_check_align_values (const char
> *flag,
> bool report_error,
> location_t loc);
>
> +extern void parse_and_check_patch_area (const char *arg, bool report_error,
> + HOST_WIDE_INT *patch_area_size,
> + HOST_WIDE_INT *patch_area_start);
> +
> extern void parse_options_from_collect_gcc_options (const char *, obstack *,
> int *);
>
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index a8947a735ff..05bd449eafc 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1725,10 +1725,14 @@ process_options (void)
> flag_sanitize &= ~SANITIZE_HWADDRESS;
> }
>
> + HOST_WIDE_INT patch_area_size, patch_area_start;
> + parse_and_check_patch_area (flag_patchable_function_entry, false,
> + &patch_area_size, &patch_area_start);
> +
> /* Do not use IPA optimizations for register allocation if profiler is
> active
> or patchable function entries are inserted for run-time instrumentation
> or port does not emit prologue and epilogue as RTL. */
> - if (profile_flag || function_entry_patch_area_size
> + if (profile_flag || patch_area_size
> || !targetm.have_prologue () || !targetm.have_epilogue ())
> flag_ipa_ra = 0;
>
> --
> 2.30.0
>
