| Message ID | 20170707193028.GA17752@suse.de |
|---|---|
| State | New |
| Headers | show |
What is the next step now? Is anybody going to commit that patch? Torsten On Fri, Jul 07, 2017 at 02:57:55PM +0100, Richard Earnshaw (lists) wrote: > On 06/07/17 15:03, Torsten Duwe wrote: > +#if TARGET_HAVE_NAMED_SECTIONS No, this is a hook. You need to test targetm_common.have_named_sections. OK with that change. R. On Fri, Jul 07, 2017 at 09:30:28PM +0200, Torsten Duwe wrote: > Change since v11: > > < +#if TARGET_HAVE_NAMED_SECTIONS > > > + if (record_p && targetm_common.have_named_sections) > > (plus > +#include "common/common-target.h" ) > > Torsten > > > gcc/c-family/ChangeLog > 2017-07-07 Torsten Duwe <duwe@suse.de> > > * c-attribs.c (c_common_attribute_table): Add entry for > "patchable_function_entry". > > gcc/lto/ChangeLog > 2017-07-07 Torsten Duwe <duwe@suse.de> > > * lto-lang.c (lto_attribute_table): Add entry for > "patchable_function_entry". > > gcc/ChangeLog > 2017-07-07 Torsten Duwe <duwe@suse.de> > > * common.opt: Introduce -fpatchable-function-entry > command line option, and its variables function_entry_patch_area_size > and function_entry_patch_area_start. > * opts.c (common_handle_option): Add -fpatchable_function_entry_ case, > including a two-value parser. > * target.def (print_patchable_function_entry): New target hook. > * targhooks.h (default_print_patchable_function_entry): New function. > * targhooks.c (default_print_patchable_function_entry): Likewise. > * toplev.c (process_options): Switch off IPA-RA if > patchable function entries are being generated. > * varasm.c (assemble_start_function): Look at the > patchable-function-entry command line switch and current > function attributes and maybe generate NOP instructions by > calling the print_patchable_function_entry hook. > * doc/extend.texi: Document patchable_function_entry attribute. > * doc/invoke.texi: Document -fpatchable_function_entry > command line option. > * doc/tm.texi.in (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): > New target hook. > * doc/tm.texi: Likewise. > > gcc/testsuite/ChangeLog > 2017-07-07 Torsten Duwe <duwe@suse.de> > > * c-c++-common/patchable_function_entry-default.c: New test. > * c-c++-common/patchable_function_entry-decl.c: Likewise. > * c-c++-common/patchable_function_entry-definition.c: Likewise. > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c > index 626ffa1cde7..ecb00c1d5b9 100644 > --- a/gcc/c-family/c-attribs.c > +++ b/gcc/c-family/c-attribs.c > @@ -142,6 +142,8 @@ static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool *) > static tree handle_bnd_legacy (tree *, tree, tree, int, bool *); > static tree handle_bnd_instrument (tree *, tree, tree, int, bool *); > static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); > +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, > + int, bool *); > > /* Table of machine-independent attributes common to all C-like languages. > > @@ -351,6 +353,9 @@ const struct attribute_spec c_common_attribute_table[] = > handle_bnd_instrument, false }, > { "fallthrough", 0, 0, false, false, false, > handle_fallthrough_attribute, false }, > + { "patchable_function_entry", 1, 2, true, false, false, > + handle_patchable_function_entry_attribute, > + false }, > { NULL, 0, 0, false, false, false, NULL, false } > }; > > @@ -3260,3 +3265,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int, > *no_add_attrs = true; > return NULL_TREE; > } > + > +static tree > +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) > +{ > + /* Nothing to be done here. */ > + return NULL_TREE; > +} > diff --git a/gcc/common.opt b/gcc/common.opt > index e81165c488b..78cfa568a95 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -163,6 +163,13 @@ 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 > @@ -2030,6 +2037,10 @@ fprofile-reorder-functions > Common Report Var(flag_profile_reorder_functions) > Enable function reordering that improves code placement. > > +fpatchable-function-entry= > +Common Joined Optimization > +Insert NOP instructions at each function entry. > + > frandom-seed > Common Var(common_deferred_options) Defer > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index 5cb512fe575..9c171abc121 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -3105,6 +3105,27 @@ that affect more than one function. > This attribute should be used for debugging purposes only. It is not > suitable in production code. > > +@item patchable_function_entry > +@cindex @code{patchable_function_entry} function attribute > +@cindex extra NOP instructions at the function entry point > +In case the target's text segment can be made writable at run time by > +any means, padding the function entry with a number of NOPs can be > +used to provide a universal tool for instrumentation. > + > +The @code{patchable_function_entry} function attribute can be used to > +change the number of NOPs to any desired value. The two-value syntax > +is the same as for the command-line switch > +@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with > +the function entry point before the @var{M}th NOP instruction. > +@var{M} defaults to 0 if omitted e.g. function entry point is before > +the first NOP. > + > +If patchable function entries are enabled globally using the command-line > +option @option{-fpatchable-function-entry=N,M}, then you must disable > +instrumentation on all functions that are part of the instrumentation > +framework with the attribute @code{patchable_function_entry (0)} > +to prevent recursion. > + > @item pure > @cindex @code{pure} function attribute > @cindex functions that have no side effects > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index d0b90503ced..dc93f0ccd50 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -11520,6 +11520,34 @@ of the function name, it is considered to be a match. For C99 and C++ > extended identifiers, the function name must be given in UTF-8, not > using universal character names. > > +@item -fpatchable-function-entry=@var{N}[,@var{M}] > +@opindex fpatchable-function-entry > +Generate @var{N} NOPs right at the beginning > +of each function, with the function entry point before the @var{M}th NOP. > +If @var{M} is omitted, it defaults to @code{0} so the > +function entry points to the address just at the first NOP. > +The NOP instructions reserve extra space which can be used to patch in > +any desired instrumentation at run time, provided that the code segment > +is writable. The amount of space is controllable indirectly via > +the number of NOPs; the NOP instruction used corresponds to the instruction > +emitted by the internal GCC back-end interface @code{gen_nop}. This behavior > +is target-specific and may also depend on the architecture variant and/or > +other compilation options. > + > +For run-time identification, the starting addresses of these areas, > +which correspond to their respective function entries minus @var{M}, > +are additionally collected in the @code{__patchable_function_entries} > +section of the resulting binary. > + > +Note that the value of @code{__attribute__ ((patchable_function_entry > +(N,M)))} takes precedence over command-line option > +@option{-fpatchable-function-entry=N,M}. This can be used to increase > +the area size or to remove it completely on a single function. > +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. > + > @end table > > > diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi > index 795e49246af..23e85c7afea 100644 > --- a/gcc/doc/tm.texi > +++ b/gcc/doc/tm.texi > @@ -4573,6 +4573,15 @@ will select the smallest suitable mode. > This section describes the macros that output function entry > (@dfn{prologue}) and exit (@dfn{epilogue}) code. > > +@deftypefn {Target Hook} void TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY (FILE *@var{file}, unsigned HOST_WIDE_INT @var{patch_area_size}, bool @var{record_p}) > +Generate a patchable area at the function start, consisting of > +@var{patch_area_size} NOP instructions. If the target supports named > +sections and if @var{record_p} is true, insert a pointer to the current > +location in the table of patchable functions. The default implementation > +of the hook places the table of pointers in the special section named > +@code{__patchable_function_entries}. > +@end deftypefn > + > @deftypefn {Target Hook} void TARGET_ASM_FUNCTION_PROLOGUE (FILE *@var{file}, HOST_WIDE_INT @var{size}) > If defined, a function that outputs the assembler code for entry to a > function. The prologue is responsible for setting up the stack frame, > diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in > index 98f2e6bce5f..6df08a2c477 100644 > --- a/gcc/doc/tm.texi.in > +++ b/gcc/doc/tm.texi.in > @@ -3650,6 +3650,8 @@ will select the smallest suitable mode. > This section describes the macros that output function entry > (@dfn{prologue}) and exit (@dfn{epilogue}) code. > > +@hook TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY > + > @hook TARGET_ASM_FUNCTION_PROLOGUE > > @hook TARGET_ASM_FUNCTION_END_PROLOGUE > diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c > index 58935172b2c..6e9a138fa3b 100644 > --- a/gcc/lto/lto-lang.c > +++ b/gcc/lto/lto-lang.c > @@ -48,6 +48,8 @@ static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *); > static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *); > static tree handle_transaction_pure_attribute (tree *, tree, tree, int, bool *); > static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *); > +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, > + int, bool *); > static tree ignore_attribute (tree *, tree, tree, int, bool *); > > static tree handle_format_attribute (tree *, tree, tree, int, bool *); > @@ -76,6 +78,9 @@ const struct attribute_spec lto_attribute_table[] = > handle_nonnull_attribute, false }, > { "nothrow", 0, 0, true, false, false, > handle_nothrow_attribute, false }, > + { "patchable_function_entry", 1, 2, true, false, false, > + handle_patchable_function_entry_attribute, > + false }, > { "returns_twice", 0, 0, true, false, false, > handle_returns_twice_attribute, false }, > { "sentinel", 0, 1, false, true, true, > @@ -473,6 +478,13 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name), > return NULL_TREE; > } > > +static tree > +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) > +{ > + /* Nothing to be done here. */ > + return NULL_TREE; > +} > + > /* Ignore the given attribute. Used when this attribute may be usefully > overridden by the target, but is not used generically. */ > > diff --git a/gcc/opts.c b/gcc/opts.c > index 7555ed55434..fecf97978fc 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -2199,6 +2199,33 @@ common_handle_option (struct gcc_options *opts, > opts->x_flag_ipa_reference = false; > break; > > + 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_start < 0 > + || function_entry_patch_area_size > + < function_entry_patch_area_start) > + error ("invalid arguments for %<-fpatchable_function_entry%>"); > + free (patch_area_arg); > + } > + break; > + > case OPT_ftree_vectorize: > if (!opts_set->x_flag_tree_loop_vectorize) > opts->x_flag_tree_loop_vectorize = value; > diff --git a/gcc/target.def b/gcc/target.def > index bbd9c015189..6d67b1fe8ba 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -288,6 +288,17 @@ hidden, protected or internal visibility as specified by @var{visibility}.", > void, (tree decl, int visibility), > default_assemble_visibility) > > +DEFHOOK > +(print_patchable_function_entry, > + "Generate a patchable area at the function start, consisting of\n\ > +@var{patch_area_size} NOP instructions. If the target supports named\n\ > +sections and if @var{record_p} is true, insert a pointer to the current\n\ > +location in the table of patchable functions. The default implementation\n\ > +of the hook places the table of pointers in the special section named\n\ > +@code{__patchable_function_entries}.", > + void, (FILE *file, unsigned HOST_WIDE_INT patch_area_size, bool record_p), > + default_print_patchable_function_entry) > + > /* Output the assembler code for entry to a function. */ > DEFHOOK > (function_prologue, > diff --git a/gcc/targhooks.c b/gcc/targhooks.c > index 940deec67f9..6a8fae656d0 100644 > --- a/gcc/targhooks.c > +++ b/gcc/targhooks.c > @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see > #include "calls.h" > #include "expr.h" > #include "output.h" > +#include "common/common-target.h" > #include "reload.h" > #include "intl.h" > #include "opts.h" > @@ -1610,6 +1611,51 @@ default_compare_by_pieces_branch_ratio (machine_mode) > return 1; > } > > +/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function > + entry. If RECORD_P is true and the target supports named sections, > + the location of the NOPs will be recorded in a special object section > + called "__patchable_function_entries". This routine may be called > + twice per function to put NOPs before and after the function > + entry. */ > + > +void > +default_print_patchable_function_entry (FILE *file, > + unsigned HOST_WIDE_INT patch_area_size, > + bool record_p) > +{ > + const char *nop_templ = 0; > + int code_num; > + rtx_insn *my_nop = make_insn_raw (gen_nop ()); > + > + /* We use the template alone, relying on the (currently sane) assumption > + that the NOP template does not have variable operands. */ > + code_num = recog_memoized (my_nop); > + nop_templ = get_insn_template (code_num, my_nop); > + > + if (record_p && targetm_common.have_named_sections) > + { > + char buf[256]; > + static int patch_area_number; > + section *previous_section = in_section; > + > + patch_area_number++; > + ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number); > + > + switch_to_section (get_section ("__patchable_function_entries", > + 0, NULL)); > + fputs (integer_asm_op (POINTER_SIZE_UNITS, false), file); > + assemble_name_raw (file, buf); > + fputc ('\n', file); > + > + switch_to_section (previous_section); > + ASM_OUTPUT_LABEL (file, buf); > + } > + > + unsigned i; > + for (i = 0; i < patch_area_size; ++i) > + fprintf (file, "\t%s\n", nop_templ); > +} > + > bool > default_profile_before_prologue (void) > { > diff --git a/gcc/targhooks.h b/gcc/targhooks.h > index 1ddb8891fe4..c73ea0bda20 100644 > --- a/gcc/targhooks.h > +++ b/gcc/targhooks.h > @@ -203,6 +203,9 @@ extern bool default_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT, > bool); > extern int default_compare_by_pieces_branch_ratio (machine_mode); > > +extern void default_print_patchable_function_entry (FILE *, > + unsigned HOST_WIDE_INT, > + bool); > extern bool default_profile_before_prologue (void); > extern reg_class_t default_preferred_reload_class (rtx, reg_class_t); > extern reg_class_t default_preferred_output_reload_class (rtx, reg_class_t); > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > new file mode 100644 > index 00000000000..8514b10e820 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 2 } } */ > + > +extern int a; > + > +/* Respect overriding attributes in the declaration. */ > +int f3 (void) __attribute__((patchable_function_entry(2))); > + > +/* F3 should now get 2 NOPs. */ > +int > +__attribute__((noinline)) > +f3 (void) > +{ > + return 5*a; > +} > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c > new file mode 100644 > index 00000000000..0dcf1181dde > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 3 } } */ > + > +extern int a; > + > +/* Nothing declared must not mean anything. */ > +int f3 (void); > + > +/* F3 should get a default-sized NOP area. */ > +int > +__attribute__((noinline)) > +f3 (void) > +{ > + return 5*a; > +} > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c > new file mode 100644 > index 00000000000..a007867dcb0 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 1 } } */ > + > +extern int a; > + > +int f3 (void); > + > +/* F3 should now get 1 NOP. */ > +int > +__attribute__((noinline)) > +__attribute__((patchable_function_entry(1))) > +f3 (void) > +{ > + return 5*a; > +} > diff --git a/gcc/toplev.c b/gcc/toplev.c > index e6c69a4ba93..b28f1847c83 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1630,8 +1630,10 @@ process_options (void) > } > > /* 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 || !targetm.have_prologue () || !targetm.have_epilogue ()) > + if (profile_flag || function_entry_patch_area_size > + || !targetm.have_prologue () || !targetm.have_epilogue ()) > flag_ipa_ra = 0; > > /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error > diff --git a/gcc/varasm.c b/gcc/varasm.c > index fbaebc1b5c0..ec1640ea378 100644 > --- a/gcc/varasm.c > +++ b/gcc/varasm.c > @@ -1825,6 +1825,46 @@ assemble_start_function (tree decl, const char *fnname) > if (DECL_PRESERVE_P (decl)) > targetm.asm_out.mark_decl_preserved (fnname); > > + 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 (decl)); > + if (patchable_function_entry_attr) > + { > + tree pp_val = TREE_VALUE (patchable_function_entry_attr); > + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); > + > + if (tree_fits_uhwi_p (patchable_function_entry_value1)) > + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); > + else > + gcc_unreachable (); > + > + patch_area_entry = 0; > + if (list_length (pp_val) > 1) > + { > + tree patchable_function_entry_value2 = > + TREE_VALUE (TREE_CHAIN (pp_val)); > + > + if (tree_fits_uhwi_p (patchable_function_entry_value2)) > + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); > + else > + gcc_unreachable (); > + } > + } > + > + if (patch_area_entry > patch_area_size) > + { > + if (patch_area_size > 0) > + warning (OPT_Wattributes, "Patchable function entry > size"); > + patch_area_entry = 0; > + } > + > + /* Emit the patching area before the entry label, if any. */ > + if (patch_area_entry > 0) > + targetm.asm_out.print_patchable_function_entry (asm_out_file, > + patch_area_entry, true); > + > /* Do any machine/system dependent processing of the function name. */ > #ifdef ASM_DECLARE_FUNCTION_NAME > ASM_DECLARE_FUNCTION_NAME (asm_out_file, fnname, current_function_decl); > @@ -1833,6 +1873,12 @@ assemble_start_function (tree decl, const char *fnname) > ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, fnname, current_function_decl); > #endif /* ASM_DECLARE_FUNCTION_NAME */ > > + /* And the area after the label. Record it if we haven't done so yet. */ > + if (patch_area_size > patch_area_entry) > + targetm.asm_out.print_patchable_function_entry (asm_out_file, > + patch_area_size-patch_area_entry, > + patch_area_entry == 0); > + > if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl))) > saw_no_split_stack = true; > }
> On Jul 17, 2017, at 3:10 PM, Torsten Duwe <duwe@suse.de> wrote: > > What is the next step now? Is anybody going to commit that patch? > > Torsten > > On Fri, Jul 07, 2017 at 02:57:55PM +0100, Richard Earnshaw (lists) wrote: >> On 06/07/17 15:03, Torsten Duwe wrote: >> +#if TARGET_HAVE_NAMED_SECTIONS > > No, this is a hook. You need to test targetm_common.have_named_sections. > > OK with that change. Torsten, if you prefer I can commit your patch (after bootstrap and a regtest on aarch64-linux-gnu and x86_64-linux-gnu). There is one last hurdle to sort out with the FSF copyright assignment. Looking at FSF's list I don't see SuSe's copyright assignment for your contributions to FSF projects, of which GCC is one. You can either request SuSe to file a copyright assignment for you, or, probably simpler, ask one of your SuSe colleagues (Richard B., Jan H.) to commit under their assignment. -- Maxim Kuvyrkov www.linaro.org > > R. > > On Fri, Jul 07, 2017 at 09:30:28PM +0200, Torsten Duwe wrote: >> Change since v11: >> >> < +#if TARGET_HAVE_NAMED_SECTIONS >> >>> + if (record_p && targetm_common.have_named_sections) >> >> (plus > +#include "common/common-target.h" ) >> >> Torsten >> >> >> gcc/c-family/ChangeLog >> 2017-07-07 Torsten Duwe <duwe@suse.de> >> >> * c-attribs.c (c_common_attribute_table): Add entry for >> "patchable_function_entry". >> >> gcc/lto/ChangeLog >> 2017-07-07 Torsten Duwe <duwe@suse.de> >> >> * lto-lang.c (lto_attribute_table): Add entry for >> "patchable_function_entry". >> >> gcc/ChangeLog >> 2017-07-07 Torsten Duwe <duwe@suse.de> >> >> * common.opt: Introduce -fpatchable-function-entry >> command line option, and its variables function_entry_patch_area_size >> and function_entry_patch_area_start. >> * opts.c (common_handle_option): Add -fpatchable_function_entry_ case, >> including a two-value parser. >> * target.def (print_patchable_function_entry): New target hook. >> * targhooks.h (default_print_patchable_function_entry): New function. >> * targhooks.c (default_print_patchable_function_entry): Likewise. >> * toplev.c (process_options): Switch off IPA-RA if >> patchable function entries are being generated. >> * varasm.c (assemble_start_function): Look at the >> patchable-function-entry command line switch and current >> function attributes and maybe generate NOP instructions by >> calling the print_patchable_function_entry hook. >> * doc/extend.texi: Document patchable_function_entry attribute. >> * doc/invoke.texi: Document -fpatchable_function_entry >> command line option. >> * doc/tm.texi.in (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): >> New target hook. >> * doc/tm.texi: Likewise. >> >> gcc/testsuite/ChangeLog >> 2017-07-07 Torsten Duwe <duwe@suse.de> >> >> * c-c++-common/patchable_function_entry-default.c: New test. >> * c-c++-common/patchable_function_entry-decl.c: Likewise. >> * c-c++-common/patchable_function_entry-definition.c: Likewise. >> >> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c >> index 626ffa1cde7..ecb00c1d5b9 100644 >> --- a/gcc/c-family/c-attribs.c >> +++ b/gcc/c-family/c-attribs.c >> @@ -142,6 +142,8 @@ static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool *) >> static tree handle_bnd_legacy (tree *, tree, tree, int, bool *); >> static tree handle_bnd_instrument (tree *, tree, tree, int, bool *); >> static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); >> +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, >> + int, bool *); >> >> /* Table of machine-independent attributes common to all C-like languages. >> >> @@ -351,6 +353,9 @@ const struct attribute_spec c_common_attribute_table[] = >> handle_bnd_instrument, false }, >> { "fallthrough", 0, 0, false, false, false, >> handle_fallthrough_attribute, false }, >> + { "patchable_function_entry", 1, 2, true, false, false, >> + handle_patchable_function_entry_attribute, >> + false }, >> { NULL, 0, 0, false, false, false, NULL, false } >> }; >> >> @@ -3260,3 +3265,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int, >> *no_add_attrs = true; >> return NULL_TREE; >> } >> + >> +static tree >> +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) >> +{ >> + /* Nothing to be done here. */ >> + return NULL_TREE; >> +} >> diff --git a/gcc/common.opt b/gcc/common.opt >> index e81165c488b..78cfa568a95 100644 >> --- a/gcc/common.opt >> +++ b/gcc/common.opt >> @@ -163,6 +163,13 @@ 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 >> @@ -2030,6 +2037,10 @@ fprofile-reorder-functions >> Common Report Var(flag_profile_reorder_functions) >> Enable function reordering that improves code placement. >> >> +fpatchable-function-entry= >> +Common Joined Optimization >> +Insert NOP instructions at each function entry. >> + >> frandom-seed >> Common Var(common_deferred_options) Defer >> >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi >> index 5cb512fe575..9c171abc121 100644 >> --- a/gcc/doc/extend.texi >> +++ b/gcc/doc/extend.texi >> @@ -3105,6 +3105,27 @@ that affect more than one function. >> This attribute should be used for debugging purposes only. It is not >> suitable in production code. >> >> +@item patchable_function_entry >> +@cindex @code{patchable_function_entry} function attribute >> +@cindex extra NOP instructions at the function entry point >> +In case the target's text segment can be made writable at run time by >> +any means, padding the function entry with a number of NOPs can be >> +used to provide a universal tool for instrumentation. >> + >> +The @code{patchable_function_entry} function attribute can be used to >> +change the number of NOPs to any desired value. The two-value syntax >> +is the same as for the command-line switch >> +@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with >> +the function entry point before the @var{M}th NOP instruction. >> +@var{M} defaults to 0 if omitted e.g. function entry point is before >> +the first NOP. >> + >> +If patchable function entries are enabled globally using the command-line >> +option @option{-fpatchable-function-entry=N,M}, then you must disable >> +instrumentation on all functions that are part of the instrumentation >> +framework with the attribute @code{patchable_function_entry (0)} >> +to prevent recursion. >> + >> @item pure >> @cindex @code{pure} function attribute >> @cindex functions that have no side effects >> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi >> index d0b90503ced..dc93f0ccd50 100644 >> --- a/gcc/doc/invoke.texi >> +++ b/gcc/doc/invoke.texi >> @@ -11520,6 +11520,34 @@ of the function name, it is considered to be a match. For C99 and C++ >> extended identifiers, the function name must be given in UTF-8, not >> using universal character names. >> >> +@item -fpatchable-function-entry=@var{N}[,@var{M}] >> +@opindex fpatchable-function-entry >> +Generate @var{N} NOPs right at the beginning >> +of each function, with the function entry point before the @var{M}th NOP. >> +If @var{M} is omitted, it defaults to @code{0} so the >> +function entry points to the address just at the first NOP. >> +The NOP instructions reserve extra space which can be used to patch in >> +any desired instrumentation at run time, provided that the code segment >> +is writable. The amount of space is controllable indirectly via >> +the number of NOPs; the NOP instruction used corresponds to the instruction >> +emitted by the internal GCC back-end interface @code{gen_nop}. This behavior >> +is target-specific and may also depend on the architecture variant and/or >> +other compilation options. >> + >> +For run-time identification, the starting addresses of these areas, >> +which correspond to their respective function entries minus @var{M}, >> +are additionally collected in the @code{__patchable_function_entries} >> +section of the resulting binary. >> + >> +Note that the value of @code{__attribute__ ((patchable_function_entry >> +(N,M)))} takes precedence over command-line option >> +@option{-fpatchable-function-entry=N,M}. This can be used to increase >> +the area size or to remove it completely on a single function. >> +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. >> + >> @end table >> >> >> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi >> index 795e49246af..23e85c7afea 100644 >> --- a/gcc/doc/tm.texi >> +++ b/gcc/doc/tm.texi >> @@ -4573,6 +4573,15 @@ will select the smallest suitable mode. >> This section describes the macros that output function entry >> (@dfn{prologue}) and exit (@dfn{epilogue}) code. >> >> +@deftypefn {Target Hook} void TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY (FILE *@var{file}, unsigned HOST_WIDE_INT @var{patch_area_size}, bool @var{record_p}) >> +Generate a patchable area at the function start, consisting of >> +@var{patch_area_size} NOP instructions. If the target supports named >> +sections and if @var{record_p} is true, insert a pointer to the current >> +location in the table of patchable functions. The default implementation >> +of the hook places the table of pointers in the special section named >> +@code{__patchable_function_entries}. >> +@end deftypefn >> + >> @deftypefn {Target Hook} void TARGET_ASM_FUNCTION_PROLOGUE (FILE *@var{file}, HOST_WIDE_INT @var{size}) >> If defined, a function that outputs the assembler code for entry to a >> function. The prologue is responsible for setting up the stack frame, >> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in >> index 98f2e6bce5f..6df08a2c477 100644 >> --- a/gcc/doc/tm.texi.in >> +++ b/gcc/doc/tm.texi.in >> @@ -3650,6 +3650,8 @@ will select the smallest suitable mode. >> This section describes the macros that output function entry >> (@dfn{prologue}) and exit (@dfn{epilogue}) code. >> >> +@hook TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY >> + >> @hook TARGET_ASM_FUNCTION_PROLOGUE >> >> @hook TARGET_ASM_FUNCTION_END_PROLOGUE >> diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c >> index 58935172b2c..6e9a138fa3b 100644 >> --- a/gcc/lto/lto-lang.c >> +++ b/gcc/lto/lto-lang.c >> @@ -48,6 +48,8 @@ static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *); >> static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *); >> static tree handle_transaction_pure_attribute (tree *, tree, tree, int, bool *); >> static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *); >> +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, >> + int, bool *); >> static tree ignore_attribute (tree *, tree, tree, int, bool *); >> >> static tree handle_format_attribute (tree *, tree, tree, int, bool *); >> @@ -76,6 +78,9 @@ const struct attribute_spec lto_attribute_table[] = >> handle_nonnull_attribute, false }, >> { "nothrow", 0, 0, true, false, false, >> handle_nothrow_attribute, false }, >> + { "patchable_function_entry", 1, 2, true, false, false, >> + handle_patchable_function_entry_attribute, >> + false }, >> { "returns_twice", 0, 0, true, false, false, >> handle_returns_twice_attribute, false }, >> { "sentinel", 0, 1, false, true, true, >> @@ -473,6 +478,13 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name), >> return NULL_TREE; >> } >> >> +static tree >> +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) >> +{ >> + /* Nothing to be done here. */ >> + return NULL_TREE; >> +} >> + >> /* Ignore the given attribute. Used when this attribute may be usefully >> overridden by the target, but is not used generically. */ >> >> diff --git a/gcc/opts.c b/gcc/opts.c >> index 7555ed55434..fecf97978fc 100644 >> --- a/gcc/opts.c >> +++ b/gcc/opts.c >> @@ -2199,6 +2199,33 @@ common_handle_option (struct gcc_options *opts, >> opts->x_flag_ipa_reference = false; >> break; >> >> + 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_start < 0 >> + || function_entry_patch_area_size >> + < function_entry_patch_area_start) >> + error ("invalid arguments for %<-fpatchable_function_entry%>"); >> + free (patch_area_arg); >> + } >> + break; >> + >> case OPT_ftree_vectorize: >> if (!opts_set->x_flag_tree_loop_vectorize) >> opts->x_flag_tree_loop_vectorize = value; >> diff --git a/gcc/target.def b/gcc/target.def >> index bbd9c015189..6d67b1fe8ba 100644 >> --- a/gcc/target.def >> +++ b/gcc/target.def >> @@ -288,6 +288,17 @@ hidden, protected or internal visibility as specified by @var{visibility}.", >> void, (tree decl, int visibility), >> default_assemble_visibility) >> >> +DEFHOOK >> +(print_patchable_function_entry, >> + "Generate a patchable area at the function start, consisting of\n\ >> +@var{patch_area_size} NOP instructions. If the target supports named\n\ >> +sections and if @var{record_p} is true, insert a pointer to the current\n\ >> +location in the table of patchable functions. The default implementation\n\ >> +of the hook places the table of pointers in the special section named\n\ >> +@code{__patchable_function_entries}.", >> + void, (FILE *file, unsigned HOST_WIDE_INT patch_area_size, bool record_p), >> + default_print_patchable_function_entry) >> + >> /* Output the assembler code for entry to a function. */ >> DEFHOOK >> (function_prologue, >> diff --git a/gcc/targhooks.c b/gcc/targhooks.c >> index 940deec67f9..6a8fae656d0 100644 >> --- a/gcc/targhooks.c >> +++ b/gcc/targhooks.c >> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see >> #include "calls.h" >> #include "expr.h" >> #include "output.h" >> +#include "common/common-target.h" >> #include "reload.h" >> #include "intl.h" >> #include "opts.h" >> @@ -1610,6 +1611,51 @@ default_compare_by_pieces_branch_ratio (machine_mode) >> return 1; >> } >> >> +/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function >> + entry. If RECORD_P is true and the target supports named sections, >> + the location of the NOPs will be recorded in a special object section >> + called "__patchable_function_entries". This routine may be called >> + twice per function to put NOPs before and after the function >> + entry. */ >> + >> +void >> +default_print_patchable_function_entry (FILE *file, >> + unsigned HOST_WIDE_INT patch_area_size, >> + bool record_p) >> +{ >> + const char *nop_templ = 0; >> + int code_num; >> + rtx_insn *my_nop = make_insn_raw (gen_nop ()); >> + >> + /* We use the template alone, relying on the (currently sane) assumption >> + that the NOP template does not have variable operands. */ >> + code_num = recog_memoized (my_nop); >> + nop_templ = get_insn_template (code_num, my_nop); >> + >> + if (record_p && targetm_common.have_named_sections) >> + { >> + char buf[256]; >> + static int patch_area_number; >> + section *previous_section = in_section; >> + >> + patch_area_number++; >> + ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number); >> + >> + switch_to_section (get_section ("__patchable_function_entries", >> + 0, NULL)); >> + fputs (integer_asm_op (POINTER_SIZE_UNITS, false), file); >> + assemble_name_raw (file, buf); >> + fputc ('\n', file); >> + >> + switch_to_section (previous_section); >> + ASM_OUTPUT_LABEL (file, buf); >> + } >> + >> + unsigned i; >> + for (i = 0; i < patch_area_size; ++i) >> + fprintf (file, "\t%s\n", nop_templ); >> +} >> + >> bool >> default_profile_before_prologue (void) >> { >> diff --git a/gcc/targhooks.h b/gcc/targhooks.h >> index 1ddb8891fe4..c73ea0bda20 100644 >> --- a/gcc/targhooks.h >> +++ b/gcc/targhooks.h >> @@ -203,6 +203,9 @@ extern bool default_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT, >> bool); >> extern int default_compare_by_pieces_branch_ratio (machine_mode); >> >> +extern void default_print_patchable_function_entry (FILE *, >> + unsigned HOST_WIDE_INT, >> + bool); >> extern bool default_profile_before_prologue (void); >> extern reg_class_t default_preferred_reload_class (rtx, reg_class_t); >> extern reg_class_t default_preferred_output_reload_class (rtx, reg_class_t); >> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >> new file mode 100644 >> index 00000000000..8514b10e820 >> --- /dev/null >> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >> @@ -0,0 +1,16 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ >> +/* { dg-final { scan-assembler-times "nop" 2 } } */ >> + >> +extern int a; >> + >> +/* Respect overriding attributes in the declaration. */ >> +int f3 (void) __attribute__((patchable_function_entry(2))); >> + >> +/* F3 should now get 2 NOPs. */ >> +int >> +__attribute__((noinline)) >> +f3 (void) >> +{ >> + return 5*a; >> +} >> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c >> new file mode 100644 >> index 00000000000..0dcf1181dde >> --- /dev/null >> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c >> @@ -0,0 +1,16 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ >> +/* { dg-final { scan-assembler-times "nop" 3 } } */ >> + >> +extern int a; >> + >> +/* Nothing declared must not mean anything. */ >> +int f3 (void); >> + >> +/* F3 should get a default-sized NOP area. */ >> +int >> +__attribute__((noinline)) >> +f3 (void) >> +{ >> + return 5*a; >> +} >> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c >> new file mode 100644 >> index 00000000000..a007867dcb0 >> --- /dev/null >> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c >> @@ -0,0 +1,16 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ >> +/* { dg-final { scan-assembler-times "nop" 1 } } */ >> + >> +extern int a; >> + >> +int f3 (void); >> + >> +/* F3 should now get 1 NOP. */ >> +int >> +__attribute__((noinline)) >> +__attribute__((patchable_function_entry(1))) >> +f3 (void) >> +{ >> + return 5*a; >> +} >> diff --git a/gcc/toplev.c b/gcc/toplev.c >> index e6c69a4ba93..b28f1847c83 100644 >> --- a/gcc/toplev.c >> +++ b/gcc/toplev.c >> @@ -1630,8 +1630,10 @@ process_options (void) >> } >> >> /* 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 || !targetm.have_prologue () || !targetm.have_epilogue ()) >> + if (profile_flag || function_entry_patch_area_size >> + || !targetm.have_prologue () || !targetm.have_epilogue ()) >> flag_ipa_ra = 0; >> >> /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error >> diff --git a/gcc/varasm.c b/gcc/varasm.c >> index fbaebc1b5c0..ec1640ea378 100644 >> --- a/gcc/varasm.c >> +++ b/gcc/varasm.c >> @@ -1825,6 +1825,46 @@ assemble_start_function (tree decl, const char *fnname) >> if (DECL_PRESERVE_P (decl)) >> targetm.asm_out.mark_decl_preserved (fnname); >> >> + 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 (decl)); >> + if (patchable_function_entry_attr) >> + { >> + tree pp_val = TREE_VALUE (patchable_function_entry_attr); >> + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); >> + >> + if (tree_fits_uhwi_p (patchable_function_entry_value1)) >> + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); >> + else >> + gcc_unreachable (); >> + >> + patch_area_entry = 0; >> + if (list_length (pp_val) > 1) >> + { >> + tree patchable_function_entry_value2 = >> + TREE_VALUE (TREE_CHAIN (pp_val)); >> + >> + if (tree_fits_uhwi_p (patchable_function_entry_value2)) >> + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); >> + else >> + gcc_unreachable (); >> + } >> + } >> + >> + if (patch_area_entry > patch_area_size) >> + { >> + if (patch_area_size > 0) >> + warning (OPT_Wattributes, "Patchable function entry > size"); >> + patch_area_entry = 0; >> + } >> + >> + /* Emit the patching area before the entry label, if any. */ >> + if (patch_area_entry > 0) >> + targetm.asm_out.print_patchable_function_entry (asm_out_file, >> + patch_area_entry, true); >> + >> /* Do any machine/system dependent processing of the function name. */ >> #ifdef ASM_DECLARE_FUNCTION_NAME >> ASM_DECLARE_FUNCTION_NAME (asm_out_file, fnname, current_function_decl); >> @@ -1833,6 +1873,12 @@ assemble_start_function (tree decl, const char *fnname) >> ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, fnname, current_function_decl); >> #endif /* ASM_DECLARE_FUNCTION_NAME */ >> >> + /* And the area after the label. Record it if we haven't done so yet. */ >> + if (patch_area_size > patch_area_entry) >> + targetm.asm_out.print_patchable_function_entry (asm_out_file, >> + patch_area_size-patch_area_entry, >> + patch_area_entry == 0); >> + >> if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl))) >> saw_no_split_stack = true; >> }
On Thu, Jul 20, 2017 at 01:58:06PM +0300, Maxim Kuvyrkov wrote: > > On Jul 17, 2017, at 3:10 PM, Torsten Duwe <duwe@suse.de> wrote: > > > > What is the next step now? Is anybody going to commit that patch? > > Torsten, if you prefer I can commit your patch (after bootstrap and a regtest on aarch64-linux-gnu and x86_64-linux-gnu). Ok, who is going to do that? > There is one last hurdle to sort out with the FSF copyright assignment. Looking at FSF's list I don't see SuSe's copyright assignment for your contributions to FSF projects, of which GCC is one. You can either request SuSe to file a copyright assignment for you, or, probably simpler, ask one of your SuSe colleagues (Richard B., Jan H.) to commit under their assignment. From my understanding, it should not make a difference, as the copyright owner is identical (SUSE). But I'm not going to argue on legal issues here. Let's first see what the test outcome is. Thanks so far! Torsten
> On Jul 20, 2017, at 3:06 PM, Torsten Duwe <duwe@suse.de> wrote: > > On Thu, Jul 20, 2017 at 01:58:06PM +0300, Maxim Kuvyrkov wrote: >>> On Jul 17, 2017, at 3:10 PM, Torsten Duwe <duwe@suse.de> wrote: >>> >>> What is the next step now? Is anybody going to commit that patch? >> >> Torsten, if you prefer I can commit your patch (after bootstrap and a regtest on aarch64-linux-gnu and x86_64-linux-gnu). > > Ok, who is going to do that? I'll do that, np. > >> There is one last hurdle to sort out with the FSF copyright assignment. Looking at FSF's list I don't see SuSe's copyright assignment for your contributions to FSF projects, of which GCC is one. You can either request SuSe to file a copyright assignment for you, or, probably simpler, ask one of your SuSe colleagues (Richard B., Jan H.) to commit under their assignment. > > From my understanding, it should not make a difference, as the copyright > owner is identical (SUSE). But I'm not going to argue on legal issues here. > Let's first see what the test outcome is. Thanks so far! SuSe/Novell choose to do its assignments per engineer, not for the whole company, so I don't think you are covered as is. -- Maxim Kuvyrkov www.linaro.org
> On Jul 20, 2017, at 4:12 PM, Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> wrote: > >> On Jul 20, 2017, at 3:06 PM, Torsten Duwe <duwe@suse.de> wrote: >> >> On Thu, Jul 20, 2017 at 01:58:06PM +0300, Maxim Kuvyrkov wrote: >>>> On Jul 17, 2017, at 3:10 PM, Torsten Duwe <duwe@suse.de> wrote: >>>> >>>> What is the next step now? Is anybody going to commit that patch? >>> >>> Torsten, if you prefer I can commit your patch (after bootstrap and a regtest on aarch64-linux-gnu and x86_64-linux-gnu). >> >> Ok, who is going to do that? > > I'll do that, np. I confirmed that bootstrap and reg test on x86_64-linux-gnu and aarch64-linux-gnu are OK. > >> >>> There is one last hurdle to sort out with the FSF copyright assignment. Looking at FSF's list I don't see SuSe's copyright assignment for your contributions to FSF projects, of which GCC is one. You can either request SuSe to file a copyright assignment for you, or, probably simpler, ask one of your SuSe colleagues (Richard B., Jan H.) to commit under their assignment. >> >> From my understanding, it should not make a difference, as the copyright >> owner is identical (SUSE). But I'm not going to argue on legal issues here. >> Let's first see what the test outcome is. Thanks so far! > > SuSe/Novell choose to do its assignments per engineer, not for the whole company, so I don't think you are covered as is. Torsten, any update on which way you are going to handle copyright assignment -- apply for a new one or have one of your SuSe colleagues commit under theirs? -- Maxim Kuvyrkov www.linaro.org
On Tue, Jul 25, 2017 at 10:12:00AM +0300, Maxim Kuvyrkov wrote: > > I confirmed that bootstrap and reg test on x86_64-linux-gnu and aarch64-linux-gnu are OK. Thanks a lot! > Torsten, any update on which way you are going to handle copyright assignment -- apply for a new one or have one of your SuSe colleagues commit under theirs? AFAIU all this code is "Copyright (C) 2016,2017 SUSE Linux GmbH", just exactly like e.g. Richard B.'s code, but IANAL. I'll find out the details. Thanks so far! Torsten
On Tue, Jul 25, 2017 at 10:12:00AM +0300, Maxim Kuvyrkov wrote: > > > > SuSe/Novell choose to do its assignments per engineer, not for the whole company, so I don't think you are covered as is. > > Torsten, any update on which way you are going to handle copyright assignment -- apply for a new one or have one of your SuSe colleagues commit under theirs? I have checked with our GCC guys and the code is covered by our company assignment. So it's safe to commit. Torsten
On Jul 07 2017, Torsten Duwe <duwe@suse.de> wrote: > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > new file mode 100644 > index 00000000000..8514b10e820 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 2 } } */ This fails on ia64. > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c > new file mode 100644 > index 00000000000..0dcf1181dde > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 3 } } */ Likewise. > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c > new file mode 100644 > index 00000000000..a007867dcb0 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c > @@ -0,0 +1,16 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > +/* { dg-final { scan-assembler-times "nop" 1 } } */ Likewise. Andreas.
On Wed, Jul 26, 2017 at 04:16:25PM +0200, Andreas Schwab wrote: > On Jul 07 2017, Torsten Duwe <duwe@suse.de> wrote: > > > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > > new file mode 100644 > > index 00000000000..8514b10e820 > > --- /dev/null > > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c > > @@ -0,0 +1,16 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ > > +/* { dg-final { scan-assembler-times "nop" 2 } } */ > > This fails on ia64. The solution is fairly obvious: on architectures where the nop is not called "nop" provide a custom, cpu-specific test, or document the failure. Torsten
On Jul 26 2017, Torsten Duwe <duwe@suse.de> wrote: > On Wed, Jul 26, 2017 at 04:16:25PM +0200, Andreas Schwab wrote: >> On Jul 07 2017, Torsten Duwe <duwe@suse.de> wrote: >> >> > diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >> > new file mode 100644 >> > index 00000000000..8514b10e820 >> > --- /dev/null >> > +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >> > @@ -0,0 +1,16 @@ >> > +/* { dg-do compile } */ >> > +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ >> > +/* { dg-final { scan-assembler-times "nop" 2 } } */ >> >> This fails on ia64. > > The solution is fairly obvious: on architectures where the nop is not called > "nop" provide a custom, cpu-specific test, or document the failure. But on ia64, a nop _is_ called nop. Andreas.
On Jul 26, 2017, at 5:33 PM, Andreas Schwab <schwab@suse.de> wrote: > > On Jul 26 2017, Torsten Duwe <duwe@suse.de> wrote: > >> On Wed, Jul 26, 2017 at 04:16:25PM +0200, Andreas Schwab wrote: >>> On Jul 07 2017, Torsten Duwe <duwe@suse.de> wrote: >>> >>>> diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >>>> new file mode 100644 >>>> index 00000000000..8514b10e820 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c >>>> @@ -0,0 +1,16 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ >>>> +/* { dg-final { scan-assembler-times "nop" 2 } } */ >>> >>> This fails on ia64. >> >> The solution is fairly obvious: on architectures where the nop is not called >> "nop" provide a custom, cpu-specific test, or document the failure. > > But on ia64, a nop _is_ called nop. The problem here is that ia64 backend emits "nop" instructions to pad IA64 bundles. The 2 nops at the beginning are [as expected] from the patchable attribute, but [unexpected] nops after ld8.mov and before "add r8" are generated by ia64 bundle packing. nop 0 nop 0 .prologue .body .mmi addl r14 = @ltoffx(a#), r1 ;; ld8.mov r14 = [r14], a# nop 0 ;; .mmi ld4 r14 = [r14] ;; shladd r8 = r14, 2, r0 nop 0 ;; .mib nop 0 add r8 = r8, r14 br.ret.sptk.many b0 I don't see an easy way to correctly differentiate between "attribute" nops and "bundle" nops, so XFAILing these tests on ia64 seems like a valid approach. I speculate that other tests fail on ia64 for the same reason, but I didn't check. -- Maxim Kuvyrkov www.linaro.org
On Mon, 31 Jul 2017, Maxim Kuvyrkov wrote: > I don't see an easy way to correctly differentiate between "attribute" > nops and "bundle" nops, so XFAILing these tests on ia64 seems like a > valid approach. Make sense, given that the use of Itanium has gone done drastically. Gerald
On Aug 01 2017, Gerald Pfeifer <gerald@pfeifer.com> wrote: > On Mon, 31 Jul 2017, Maxim Kuvyrkov wrote: >> I don't see an easy way to correctly differentiate between "attribute" >> nops and "bundle" nops, so XFAILing these tests on ia64 seems like a >> valid approach. > > Make sense, given that the use of Itanium has gone done drastically. You can get the same failure with any target, for example if there are delay slots to be filled. Andreas.
> On Aug 1, 2017, at 1:52 PM, Andreas Schwab <schwab@suse.de> wrote: > > On Aug 01 2017, Gerald Pfeifer <gerald@pfeifer.com> wrote: > >> On Mon, 31 Jul 2017, Maxim Kuvyrkov wrote: >>> I don't see an easy way to correctly differentiate between "attribute" >>> nops and "bundle" nops, so XFAILing these tests on ia64 seems like a >>> valid approach. >> >> Make sense, given that the use of Itanium has gone done drastically. > > You can get the same failure with any target, for example if there are > delay slots to be filled. Andreas, Do you know a reliable way of checking whether target can issue nops in simple code? One alternative would be to apply testcase only to white-listed architectures, which is, imo, less preferable. Regards, -- Maxim Kuvyrkov www.linaro.org
On Aug 01 2017, Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org> wrote:
> Do you know a reliable way of checking whether target can issue nops in simple code?
Try inspecting one of the rtl dumps.
Andreas.
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 626ffa1cde7..ecb00c1d5b9 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -142,6 +142,8 @@ static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool *) static tree handle_bnd_legacy (tree *, tree, tree, int, bool *); static tree handle_bnd_instrument (tree *, tree, tree, int, bool *); static tree handle_fallthrough_attribute (tree *, tree, tree, int, bool *); +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, + int, bool *); /* Table of machine-independent attributes common to all C-like languages. @@ -351,6 +353,9 @@ const struct attribute_spec c_common_attribute_table[] = handle_bnd_instrument, false }, { "fallthrough", 0, 0, false, false, false, handle_fallthrough_attribute, false }, + { "patchable_function_entry", 1, 2, true, false, false, + handle_patchable_function_entry_attribute, + false }, { NULL, 0, 0, false, false, false, NULL, false } }; @@ -3260,3 +3265,10 @@ handle_fallthrough_attribute (tree *, tree name, tree, int, *no_add_attrs = true; return NULL_TREE; } + +static tree +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) +{ + /* Nothing to be done here. */ + return NULL_TREE; +} diff --git a/gcc/common.opt b/gcc/common.opt index e81165c488b..78cfa568a95 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -163,6 +163,13 @@ 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 @@ -2030,6 +2037,10 @@ fprofile-reorder-functions Common Report Var(flag_profile_reorder_functions) Enable function reordering that improves code placement. +fpatchable-function-entry= +Common Joined Optimization +Insert NOP instructions at each function entry. + frandom-seed Common Var(common_deferred_options) Defer diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 5cb512fe575..9c171abc121 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -3105,6 +3105,27 @@ that affect more than one function. This attribute should be used for debugging purposes only. It is not suitable in production code. +@item patchable_function_entry +@cindex @code{patchable_function_entry} function attribute +@cindex extra NOP instructions at the function entry point +In case the target's text segment can be made writable at run time by +any means, padding the function entry with a number of NOPs can be +used to provide a universal tool for instrumentation. + +The @code{patchable_function_entry} function attribute can be used to +change the number of NOPs to any desired value. The two-value syntax +is the same as for the command-line switch +@option{-fpatchable-function-entry=N,M}, generating @var{N} NOPs, with +the function entry point before the @var{M}th NOP instruction. +@var{M} defaults to 0 if omitted e.g. function entry point is before +the first NOP. + +If patchable function entries are enabled globally using the command-line +option @option{-fpatchable-function-entry=N,M}, then you must disable +instrumentation on all functions that are part of the instrumentation +framework with the attribute @code{patchable_function_entry (0)} +to prevent recursion. + @item pure @cindex @code{pure} function attribute @cindex functions that have no side effects diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index d0b90503ced..dc93f0ccd50 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -11520,6 +11520,34 @@ of the function name, it is considered to be a match. For C99 and C++ extended identifiers, the function name must be given in UTF-8, not using universal character names. +@item -fpatchable-function-entry=@var{N}[,@var{M}] +@opindex fpatchable-function-entry +Generate @var{N} NOPs right at the beginning +of each function, with the function entry point before the @var{M}th NOP. +If @var{M} is omitted, it defaults to @code{0} so the +function entry points to the address just at the first NOP. +The NOP instructions reserve extra space which can be used to patch in +any desired instrumentation at run time, provided that the code segment +is writable. The amount of space is controllable indirectly via +the number of NOPs; the NOP instruction used corresponds to the instruction +emitted by the internal GCC back-end interface @code{gen_nop}. This behavior +is target-specific and may also depend on the architecture variant and/or +other compilation options. + +For run-time identification, the starting addresses of these areas, +which correspond to their respective function entries minus @var{M}, +are additionally collected in the @code{__patchable_function_entries} +section of the resulting binary. + +Note that the value of @code{__attribute__ ((patchable_function_entry +(N,M)))} takes precedence over command-line option +@option{-fpatchable-function-entry=N,M}. This can be used to increase +the area size or to remove it completely on a single function. +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. + @end table diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 795e49246af..23e85c7afea 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -4573,6 +4573,15 @@ will select the smallest suitable mode. This section describes the macros that output function entry (@dfn{prologue}) and exit (@dfn{epilogue}) code. +@deftypefn {Target Hook} void TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY (FILE *@var{file}, unsigned HOST_WIDE_INT @var{patch_area_size}, bool @var{record_p}) +Generate a patchable area at the function start, consisting of +@var{patch_area_size} NOP instructions. If the target supports named +sections and if @var{record_p} is true, insert a pointer to the current +location in the table of patchable functions. The default implementation +of the hook places the table of pointers in the special section named +@code{__patchable_function_entries}. +@end deftypefn + @deftypefn {Target Hook} void TARGET_ASM_FUNCTION_PROLOGUE (FILE *@var{file}, HOST_WIDE_INT @var{size}) If defined, a function that outputs the assembler code for entry to a function. The prologue is responsible for setting up the stack frame, diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index 98f2e6bce5f..6df08a2c477 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -3650,6 +3650,8 @@ will select the smallest suitable mode. This section describes the macros that output function entry (@dfn{prologue}) and exit (@dfn{epilogue}) code. +@hook TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY + @hook TARGET_ASM_FUNCTION_PROLOGUE @hook TARGET_ASM_FUNCTION_END_PROLOGUE diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c index 58935172b2c..6e9a138fa3b 100644 --- a/gcc/lto/lto-lang.c +++ b/gcc/lto/lto-lang.c @@ -48,6 +48,8 @@ static tree handle_sentinel_attribute (tree *, tree, tree, int, bool *); static tree handle_type_generic_attribute (tree *, tree, tree, int, bool *); static tree handle_transaction_pure_attribute (tree *, tree, tree, int, bool *); static tree handle_returns_twice_attribute (tree *, tree, tree, int, bool *); +static tree handle_patchable_function_entry_attribute (tree *, tree, tree, + int, bool *); static tree ignore_attribute (tree *, tree, tree, int, bool *); static tree handle_format_attribute (tree *, tree, tree, int, bool *); @@ -76,6 +78,9 @@ const struct attribute_spec lto_attribute_table[] = handle_nonnull_attribute, false }, { "nothrow", 0, 0, true, false, false, handle_nothrow_attribute, false }, + { "patchable_function_entry", 1, 2, true, false, false, + handle_patchable_function_entry_attribute, + false }, { "returns_twice", 0, 0, true, false, false, handle_returns_twice_attribute, false }, { "sentinel", 0, 1, false, true, true, @@ -473,6 +478,13 @@ handle_returns_twice_attribute (tree *node, tree ARG_UNUSED (name), return NULL_TREE; } +static tree +handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *) +{ + /* Nothing to be done here. */ + return NULL_TREE; +} + /* Ignore the given attribute. Used when this attribute may be usefully overridden by the target, but is not used generically. */ diff --git a/gcc/opts.c b/gcc/opts.c index 7555ed55434..fecf97978fc 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2199,6 +2199,33 @@ common_handle_option (struct gcc_options *opts, opts->x_flag_ipa_reference = false; break; + 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_start < 0 + || function_entry_patch_area_size + < function_entry_patch_area_start) + error ("invalid arguments for %<-fpatchable_function_entry%>"); + free (patch_area_arg); + } + break; + case OPT_ftree_vectorize: if (!opts_set->x_flag_tree_loop_vectorize) opts->x_flag_tree_loop_vectorize = value; diff --git a/gcc/target.def b/gcc/target.def index bbd9c015189..6d67b1fe8ba 100644 --- a/gcc/target.def +++ b/gcc/target.def @@ -288,6 +288,17 @@ hidden, protected or internal visibility as specified by @var{visibility}.", void, (tree decl, int visibility), default_assemble_visibility) +DEFHOOK +(print_patchable_function_entry, + "Generate a patchable area at the function start, consisting of\n\ +@var{patch_area_size} NOP instructions. If the target supports named\n\ +sections and if @var{record_p} is true, insert a pointer to the current\n\ +location in the table of patchable functions. The default implementation\n\ +of the hook places the table of pointers in the special section named\n\ +@code{__patchable_function_entries}.", + void, (FILE *file, unsigned HOST_WIDE_INT patch_area_size, bool record_p), + default_print_patchable_function_entry) + /* Output the assembler code for entry to a function. */ DEFHOOK (function_prologue, diff --git a/gcc/targhooks.c b/gcc/targhooks.c index 940deec67f9..6a8fae656d0 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see #include "calls.h" #include "expr.h" #include "output.h" +#include "common/common-target.h" #include "reload.h" #include "intl.h" #include "opts.h" @@ -1610,6 +1611,51 @@ default_compare_by_pieces_branch_ratio (machine_mode) return 1; } +/* Write PATCH_AREA_SIZE NOPs into the asm outfile FILE around a function + entry. If RECORD_P is true and the target supports named sections, + the location of the NOPs will be recorded in a special object section + called "__patchable_function_entries". This routine may be called + twice per function to put NOPs before and after the function + entry. */ + +void +default_print_patchable_function_entry (FILE *file, + unsigned HOST_WIDE_INT patch_area_size, + bool record_p) +{ + const char *nop_templ = 0; + int code_num; + rtx_insn *my_nop = make_insn_raw (gen_nop ()); + + /* We use the template alone, relying on the (currently sane) assumption + that the NOP template does not have variable operands. */ + code_num = recog_memoized (my_nop); + nop_templ = get_insn_template (code_num, my_nop); + + if (record_p && targetm_common.have_named_sections) + { + char buf[256]; + static int patch_area_number; + section *previous_section = in_section; + + patch_area_number++; + ASM_GENERATE_INTERNAL_LABEL (buf, "LPFE", patch_area_number); + + switch_to_section (get_section ("__patchable_function_entries", + 0, NULL)); + fputs (integer_asm_op (POINTER_SIZE_UNITS, false), file); + assemble_name_raw (file, buf); + fputc ('\n', file); + + switch_to_section (previous_section); + ASM_OUTPUT_LABEL (file, buf); + } + + unsigned i; + for (i = 0; i < patch_area_size; ++i) + fprintf (file, "\t%s\n", nop_templ); +} + bool default_profile_before_prologue (void) { diff --git a/gcc/targhooks.h b/gcc/targhooks.h index 1ddb8891fe4..c73ea0bda20 100644 --- a/gcc/targhooks.h +++ b/gcc/targhooks.h @@ -203,6 +203,9 @@ extern bool default_use_by_pieces_infrastructure_p (unsigned HOST_WIDE_INT, bool); extern int default_compare_by_pieces_branch_ratio (machine_mode); +extern void default_print_patchable_function_entry (FILE *, + unsigned HOST_WIDE_INT, + bool); extern bool default_profile_before_prologue (void); extern reg_class_t default_preferred_reload_class (rtx, reg_class_t); extern reg_class_t default_preferred_output_reload_class (rtx, reg_class_t); diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c new file mode 100644 index 00000000000..8514b10e820 --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ +/* { dg-final { scan-assembler-times "nop" 2 } } */ + +extern int a; + +/* Respect overriding attributes in the declaration. */ +int f3 (void) __attribute__((patchable_function_entry(2))); + +/* F3 should now get 2 NOPs. */ +int +__attribute__((noinline)) +f3 (void) +{ + return 5*a; +} diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c new file mode 100644 index 00000000000..0dcf1181dde --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ +/* { dg-final { scan-assembler-times "nop" 3 } } */ + +extern int a; + +/* Nothing declared must not mean anything. */ +int f3 (void); + +/* F3 should get a default-sized NOP area. */ +int +__attribute__((noinline)) +f3 (void) +{ + return 5*a; +} diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c new file mode 100644 index 00000000000..a007867dcb0 --- /dev/null +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ +/* { dg-final { scan-assembler-times "nop" 1 } } */ + +extern int a; + +int f3 (void); + +/* F3 should now get 1 NOP. */ +int +__attribute__((noinline)) +__attribute__((patchable_function_entry(1))) +f3 (void) +{ + return 5*a; +} diff --git a/gcc/toplev.c b/gcc/toplev.c index e6c69a4ba93..b28f1847c83 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1630,8 +1630,10 @@ process_options (void) } /* 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 || !targetm.have_prologue () || !targetm.have_epilogue ()) + if (profile_flag || function_entry_patch_area_size + || !targetm.have_prologue () || !targetm.have_epilogue ()) flag_ipa_ra = 0; /* Enable -Werror=coverage-mismatch when -Werror and -Wno-error diff --git a/gcc/varasm.c b/gcc/varasm.c index fbaebc1b5c0..ec1640ea378 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -1825,6 +1825,46 @@ assemble_start_function (tree decl, const char *fnname) if (DECL_PRESERVE_P (decl)) targetm.asm_out.mark_decl_preserved (fnname); + 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 (decl)); + if (patchable_function_entry_attr) + { + tree pp_val = TREE_VALUE (patchable_function_entry_attr); + tree patchable_function_entry_value1 = TREE_VALUE (pp_val); + + if (tree_fits_uhwi_p (patchable_function_entry_value1)) + patch_area_size = tree_to_uhwi (patchable_function_entry_value1); + else + gcc_unreachable (); + + patch_area_entry = 0; + if (list_length (pp_val) > 1) + { + tree patchable_function_entry_value2 = + TREE_VALUE (TREE_CHAIN (pp_val)); + + if (tree_fits_uhwi_p (patchable_function_entry_value2)) + patch_area_entry = tree_to_uhwi (patchable_function_entry_value2); + else + gcc_unreachable (); + } + } + + if (patch_area_entry > patch_area_size) + { + if (patch_area_size > 0) + warning (OPT_Wattributes, "Patchable function entry > size"); + patch_area_entry = 0; + } + + /* Emit the patching area before the entry label, if any. */ + if (patch_area_entry > 0) + targetm.asm_out.print_patchable_function_entry (asm_out_file, + patch_area_entry, true); + /* Do any machine/system dependent processing of the function name. */ #ifdef ASM_DECLARE_FUNCTION_NAME ASM_DECLARE_FUNCTION_NAME (asm_out_file, fnname, current_function_decl); @@ -1833,6 +1873,12 @@ assemble_start_function (tree decl, const char *fnname) ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, fnname, current_function_decl); #endif /* ASM_DECLARE_FUNCTION_NAME */ + /* And the area after the label. Record it if we haven't done so yet. */ + if (patch_area_size > patch_area_entry) + targetm.asm_out.print_patchable_function_entry (asm_out_file, + patch_area_size-patch_area_entry, + patch_area_entry == 0); + if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl))) saw_no_split_stack = true; }
Change since v11: < +#if TARGET_HAVE_NAMED_SECTIONS > + if (record_p && targetm_common.have_named_sections) (plus > +#include "common/common-target.h" ) Torsten gcc/c-family/ChangeLog 2017-07-07 Torsten Duwe <duwe@suse.de> * c-attribs.c (c_common_attribute_table): Add entry for "patchable_function_entry". gcc/lto/ChangeLog 2017-07-07 Torsten Duwe <duwe@suse.de> * lto-lang.c (lto_attribute_table): Add entry for "patchable_function_entry". gcc/ChangeLog 2017-07-07 Torsten Duwe <duwe@suse.de> * common.opt: Introduce -fpatchable-function-entry command line option, and its variables function_entry_patch_area_size and function_entry_patch_area_start. * opts.c (common_handle_option): Add -fpatchable_function_entry_ case, including a two-value parser. * target.def (print_patchable_function_entry): New target hook. * targhooks.h (default_print_patchable_function_entry): New function. * targhooks.c (default_print_patchable_function_entry): Likewise. * toplev.c (process_options): Switch off IPA-RA if patchable function entries are being generated. * varasm.c (assemble_start_function): Look at the patchable-function-entry command line switch and current function attributes and maybe generate NOP instructions by calling the print_patchable_function_entry hook. * doc/extend.texi: Document patchable_function_entry attribute. * doc/invoke.texi: Document -fpatchable_function_entry command line option. * doc/tm.texi.in (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New target hook. * doc/tm.texi: Likewise. gcc/testsuite/ChangeLog 2017-07-07 Torsten Duwe <duwe@suse.de> * c-c++-common/patchable_function_entry-default.c: New test. * c-c++-common/patchable_function_entry-decl.c: Likewise. * c-c++-common/patchable_function_entry-definition.c: Likewise.