"H.J. Lu" <hjl.to...@gmail.com> writes:
> Currently patchable area is at the wrong place.
Agreed :-)
> It is placed immediately
> after function label and before .cfi_startproc. A backend should be able
> to add a pseudo patchable area instruction durectly into RTL. This patch
> adds patch_area_size and patch_area_entry to cfun so that the patchable
> area info is available in RTL passes.
It might be better to add it to crtl, since it should only be needed
during rtl generation.
> It also limits patch_area_size and patch_area_entry to 65535, which is
> a reasonable maximum size for patchable area.
>
> gcc/
>
> PR target/93492
> * function.c (expand_function_start): Set cfun->patch_area_size
> and cfun->patch_area_entry.
> * function.h (function): Add patch_area_size and patch_area_entry.
> * opts.c (common_handle_option): Limit
> function_entry_patch_area_size and function_entry_patch_area_start
> to USHRT_MAX. Fix a typo in error message.
> * varasm.c (assemble_start_function): Use cfun->patch_area_size
> and cfun->patch_area_entry.
> * doc/invoke.texi: Document the maximum value for
> -fpatchable-function-entry.
>
> gcc/testsuite/
>
> PR target/93492
> * c-c++-common/patchable_function_entry-error-1.c: New test.
> * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> * c-c++-common/patchable_function_entry-error-3.c: Likewise.
> ---
> gcc/doc/invoke.texi | 1 +
> gcc/function.c | 35 +++++++++++++++++++
> gcc/function.h | 6 ++++
> gcc/opts.c | 4 ++-
> .../patchable_function_entry-error-1.c | 9 +++++
> .../patchable_function_entry-error-2.c | 9 +++++
> .../patchable_function_entry-error-3.c | 20 +++++++++++
> gcc/varasm.c | 30 ++--------------
> 8 files changed, 85 insertions(+), 29 deletions(-)
> create mode 100644
> gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
> create mode 100644
> gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
> create mode 100644
> gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 35b341e759f..dd4835199b0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
> The NOP instructions are inserted at---and maybe before, depending on
> @var{M}---the function entry address, even before the prologue.
>
> +The maximum value of @var{N} and @var{M} is 65535.
> @end table
>
>
> diff --git a/gcc/function.c b/gcc/function.c
> index d8008f60422..badbf538eec 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
> /* If we are doing generic stack checking, the probe should go here. */
> if (flag_stack_check == GENERIC_STACK_CHECK)
> stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> +
> + unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> + unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> +
> + tree patchable_function_entry_attr
> + = lookup_attribute ("patchable_function_entry",
> + DECL_ATTRIBUTES (cfun->decl));
> + if (patchable_function_entry_attr)
> + {
> + tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> + tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> +
> + patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> + patch_area_entry = 0;
> + if (TREE_CHAIN (pp_val) != NULL_TREE)
> + {
> + tree patchable_function_entry_value2
> + = TREE_VALUE (TREE_CHAIN (pp_val));
> + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> + }
> + if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> + error ("invalid values for %<patchable_function_entry%> attribute");
This should probably go in handle_patchable_function_entry_attribute
instead. It doesn't look like we have a clear policy about whether
errors or warnings are right for unrecognised arguments to known
attribute names, but handle_patchable_function_entry_attribute reports
an OPT_Wattributes warning for arguments that aren't integers. Doing the
same for out-of-range integers would be more consistent and means that
we wouldn't break existing code if we relaxed/changed the rules in future.
Thanks,
Richard
