* [PATCH v11] add -fpatchable-function-entry=N,M option @ 2017-07-06 14:03 Torsten Duwe 2017-07-07 13:58 ` Richard Earnshaw (lists) 0 siblings, 1 reply; 20+ messages in thread From: Torsten Duwe @ 2017-07-06 14:03 UTC (permalink / raw) To: Richard Earnshaw (lists) Cc: Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa Permit A 38 gcc/c-family/ChangeLog 2017-07-06 Torsten Duwe <duwe@suse.de> * c-attribs.c (c_common_attribute_table): Add entry for "patchable_function_entry". gcc/lto/ChangeLog 2017-07-06 Torsten Duwe <duwe@suse.de> * lto-lang.c (lto_attribute_table): Add entry for "patchable_function_entry". gcc/ChangeLog 2017-07-06 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-06 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 03ba8fc436c..86d567783f7 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 3e5cee8649e..be93d401dff 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 7460c2be1b6..10c52976c90 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -2196,6 +2196,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..725d8f3e734 100644 --- a/gcc/targhooks.c +++ b/gcc/targhooks.c @@ -1610,6 +1610,53 @@ 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) + { +#if TARGET_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); +#endif + } + + 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; } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v11] add -fpatchable-function-entry=N,M option 2017-07-06 14:03 [PATCH v11] add -fpatchable-function-entry=N,M option Torsten Duwe @ 2017-07-07 13:58 ` Richard Earnshaw (lists) 2017-07-07 19:30 ` [PATCH v12] " Torsten Duwe 0 siblings, 1 reply; 20+ messages in thread From: Richard Earnshaw (lists) @ 2017-07-07 13:58 UTC (permalink / raw) To: Torsten Duwe Cc: Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa On 06/07/17 15:03, Torsten Duwe wrote: > Permit A 38 > > gcc/c-family/ChangeLog > 2017-07-06 Torsten Duwe <duwe@suse.de> > > * c-attribs.c (c_common_attribute_table): Add entry for > "patchable_function_entry". > > gcc/lto/ChangeLog > 2017-07-06 Torsten Duwe <duwe@suse.de> > > * lto-lang.c (lto_attribute_table): Add entry for > "patchable_function_entry". > > gcc/ChangeLog > 2017-07-06 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-06 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 03ba8fc436c..86d567783f7 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 3e5cee8649e..be93d401dff 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 7460c2be1b6..10c52976c90 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -2196,6 +2196,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..725d8f3e734 100644 > --- a/gcc/targhooks.c > +++ b/gcc/targhooks.c > @@ -1610,6 +1610,53 @@ 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) > + { > +#if TARGET_HAVE_NAMED_SECTIONS No, this is a hook. You need to test targetm_common.have_named_sections. OK with that change. R. > + 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); > +#endif > + } > + > + 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; > } > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-07 13:58 ` Richard Earnshaw (lists) @ 2017-07-07 19:30 ` Torsten Duwe 2017-07-17 12:10 ` Torsten Duwe 2017-07-26 14:16 ` Andreas Schwab 0 siblings, 2 replies; 20+ messages in thread From: Torsten Duwe @ 2017-07-07 19:30 UTC (permalink / raw) To: Richard Earnshaw (lists) Cc: Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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; } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-07 19:30 ` [PATCH v12] " Torsten Duwe @ 2017-07-17 12:10 ` Torsten Duwe 2017-07-20 10:58 ` Maxim Kuvyrkov 2017-07-26 14:16 ` Andreas Schwab 1 sibling, 1 reply; 20+ messages in thread From: Torsten Duwe @ 2017-07-17 12:10 UTC (permalink / raw) To: Richard Earnshaw (lists) Cc: Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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; > } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-17 12:10 ` Torsten Duwe @ 2017-07-20 10:58 ` Maxim Kuvyrkov 2017-07-20 12:06 ` Torsten Duwe 0 siblings, 1 reply; 20+ messages in thread From: Maxim Kuvyrkov @ 2017-07-20 10:58 UTC (permalink / raw) To: Torsten Duwe, Richard Biener, Jan Hubicka Cc: Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa > 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; >> } ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-20 10:58 ` Maxim Kuvyrkov @ 2017-07-20 12:06 ` Torsten Duwe 2017-07-20 13:12 ` Maxim Kuvyrkov 0 siblings, 1 reply; 20+ messages in thread From: Torsten Duwe @ 2017-07-20 12:06 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Richard Biener, Jan Hubicka, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-20 12:06 ` Torsten Duwe @ 2017-07-20 13:12 ` Maxim Kuvyrkov 2017-07-25 7:16 ` Maxim Kuvyrkov 0 siblings, 1 reply; 20+ messages in thread From: Maxim Kuvyrkov @ 2017-07-20 13:12 UTC (permalink / raw) To: Torsten Duwe Cc: Richard Guenther, Jan Hubicka, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-20 13:12 ` Maxim Kuvyrkov @ 2017-07-25 7:16 ` Maxim Kuvyrkov 2017-07-25 8:07 ` Torsten Duwe 2017-07-25 13:03 ` Torsten Duwe 0 siblings, 2 replies; 20+ messages in thread From: Maxim Kuvyrkov @ 2017-07-25 7:16 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Torsten Duwe, Richard Guenther, Jan Hubicka, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-25 7:16 ` Maxim Kuvyrkov @ 2017-07-25 8:07 ` Torsten Duwe 2017-07-25 13:03 ` Torsten Duwe 1 sibling, 0 replies; 20+ messages in thread From: Torsten Duwe @ 2017-07-25 8:07 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Richard Guenther, Jan Hubicka, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-25 7:16 ` Maxim Kuvyrkov 2017-07-25 8:07 ` Torsten Duwe @ 2017-07-25 13:03 ` Torsten Duwe 1 sibling, 0 replies; 20+ messages in thread From: Torsten Duwe @ 2017-07-25 13:03 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Richard Guenther, Jan Hubicka, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-07 19:30 ` [PATCH v12] " Torsten Duwe 2017-07-17 12:10 ` Torsten Duwe @ 2017-07-26 14:16 ` Andreas Schwab 2017-07-26 14:26 ` Torsten Duwe 1 sibling, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2017-07-26 14:16 UTC (permalink / raw) To: Torsten Duwe Cc: Richard Earnshaw (lists), Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-26 14:16 ` Andreas Schwab @ 2017-07-26 14:26 ` Torsten Duwe 2017-07-26 14:33 ` Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Torsten Duwe @ 2017-07-26 14:26 UTC (permalink / raw) To: Andreas Schwab Cc: Richard Earnshaw (lists), Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-26 14:26 ` Torsten Duwe @ 2017-07-26 14:33 ` Andreas Schwab 2017-07-31 11:47 ` Maxim Kuvyrkov 0 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2017-07-26 14:33 UTC (permalink / raw) To: Torsten Duwe Cc: Richard Earnshaw (lists), Sandra Loosemore, Maxim Kuvyrkov, Marek Polacek, Maxim Kuvyrkov, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-26 14:33 ` Andreas Schwab @ 2017-07-31 11:47 ` Maxim Kuvyrkov 2017-08-01 10:43 ` Gerald Pfeifer 0 siblings, 1 reply; 20+ messages in thread From: Maxim Kuvyrkov @ 2017-07-31 11:47 UTC (permalink / raw) To: Andreas Schwab Cc: Torsten Duwe, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-07-31 11:47 ` Maxim Kuvyrkov @ 2017-08-01 10:43 ` Gerald Pfeifer 2017-08-01 10:53 ` Andreas Schwab 0 siblings, 1 reply; 20+ messages in thread From: Gerald Pfeifer @ 2017-08-01 10:43 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Andreas Schwab, Torsten Duwe, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-08-01 10:43 ` Gerald Pfeifer @ 2017-08-01 10:53 ` Andreas Schwab 2017-08-01 13:03 ` Maxim Kuvyrkov 0 siblings, 1 reply; 20+ messages in thread From: Andreas Schwab @ 2017-08-01 10:53 UTC (permalink / raw) To: Gerald Pfeifer Cc: Maxim Kuvyrkov, Torsten Duwe, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-08-01 10:53 ` Andreas Schwab @ 2017-08-01 13:03 ` Maxim Kuvyrkov 2017-08-01 13:36 ` Andreas Schwab 2017-08-11 12:42 ` [PATCH] skip patchable_function_entry tests on ia64 Torsten Duwe 0 siblings, 2 replies; 20+ messages in thread From: Maxim Kuvyrkov @ 2017-08-01 13:03 UTC (permalink / raw) To: Andreas Schwab Cc: Gerald Pfeifer, Torsten Duwe, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v12] add -fpatchable-function-entry=N,M option 2017-08-01 13:03 ` Maxim Kuvyrkov @ 2017-08-01 13:36 ` Andreas Schwab 2017-08-11 12:42 ` [PATCH] skip patchable_function_entry tests on ia64 Torsten Duwe 1 sibling, 0 replies; 20+ messages in thread From: Andreas Schwab @ 2017-08-01 13:36 UTC (permalink / raw) To: Maxim Kuvyrkov Cc: Gerald Pfeifer, Torsten Duwe, Richard Earnshaw (lists), Sandra Loosemore, Marek Polacek, GCC Patches, Szabolcs Nagy, nd, Li Bin, Jiri Kosina, Marcus Shawcroft, Takahiro Akashi, Andrew Wafaa 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. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] skip patchable_function_entry tests on ia64 2017-08-01 13:03 ` Maxim Kuvyrkov 2017-08-01 13:36 ` Andreas Schwab @ 2017-08-11 12:42 ` Torsten Duwe 2017-08-14 16:50 ` Jim Wilson 1 sibling, 1 reply; 20+ messages in thread From: Torsten Duwe @ 2017-08-11 12:42 UTC (permalink / raw) To: Jim Wilson; +Cc: Andreas Schwab, Maxim Kuvyrkov, GCC Patches As Andreas found out, patchable_function_entry tests generate false failures on ia64. On a closer look, this feature needs some more thought on itanium: I *could* imagine it is a good idea to generate N complete nop "bundles", which would be properly aligned and allow for easy replacement with alternate binary code. But I'd rather leave this to the platform experts, or whoever comes up with a good use case, to implement the proper cpu override hook. Meanwhile, as long as the expected results remain undefined, disable the tests. Torsten gcc/testsuite/ChangeLog 2017-08-11 Torsten Duwe <duwe@suse.de> * c-c++-common/patchable_function_entry-default.c: Skip test on ia64. * c-c++-common/patchable_function_entry-decl.c: Likewise. * c-c++-common/patchable_function_entry-definition.c: Likewise. diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c index 5c39a354559..e35dce4f3d8 100644 --- a/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-decl.c @@ -1,4 +1,5 @@ /* { dg-do compile { target { ! nvptx*-*-* } } } */ +/* { dg-skip-if "undefined padding" { "ia64*-*-*" } { "*" } { "" } } */ /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ /* { dg-final { scan-assembler-times "nop" 2 } } */ diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c index 48094f75f78..1b7e188d5f9 100644 --- a/gcc/testsuite/c-c++-common/patchable_function_entry-default.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-default.c @@ -1,4 +1,5 @@ /* { dg-do compile { target { ! nvptx*-*-* } } } */ +/* { dg-skip-if "undefined padding" { "ia64*-*-*" } { "*" } { "" } } */ /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ /* { dg-final { scan-assembler-times "nop" 3 } } */ diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c index af8202f283b..33e37dbc49f 100644 --- a/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c +++ b/gcc/testsuite/c-c++-common/patchable_function_entry-definition.c @@ -1,4 +1,5 @@ /* { dg-do compile { target { ! nvptx*-*-* } } } */ +/* { dg-skip-if "undefined padding" { "ia64*-*-*" } { "*" } { "" } } */ /* { dg-options "-O2 -fpatchable-function-entry=3,1" } */ /* { dg-final { scan-assembler-times "nop" 1 } } */ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] skip patchable_function_entry tests on ia64 2017-08-11 12:42 ` [PATCH] skip patchable_function_entry tests on ia64 Torsten Duwe @ 2017-08-14 16:50 ` Jim Wilson 0 siblings, 0 replies; 20+ messages in thread From: Jim Wilson @ 2017-08-14 16:50 UTC (permalink / raw) To: Torsten Duwe; +Cc: Andreas Schwab, Maxim Kuvyrkov, GCC Patches On Fri, 2017-08-11 at 12:34 +0200, Torsten Duwe wrote: > gcc/testsuite/ChangeLog > 2017-08-11 Torsten Duwe <duwe@suse.de> > > * c-c++-common/patchable_function_entry-default.c: Skip test on > ia64. > * c-c++-common/patchable_function_entry-decl.c: Likewise. > * c-c++-common/patchable_function_entry-definition.c: Likewise. OK. Jim ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-08-14 15:10 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-06 14:03 [PATCH v11] add -fpatchable-function-entry=N,M option Torsten Duwe 2017-07-07 13:58 ` Richard Earnshaw (lists) 2017-07-07 19:30 ` [PATCH v12] " Torsten Duwe 2017-07-17 12:10 ` Torsten Duwe 2017-07-20 10:58 ` Maxim Kuvyrkov 2017-07-20 12:06 ` Torsten Duwe 2017-07-20 13:12 ` Maxim Kuvyrkov 2017-07-25 7:16 ` Maxim Kuvyrkov 2017-07-25 8:07 ` Torsten Duwe 2017-07-25 13:03 ` Torsten Duwe 2017-07-26 14:16 ` Andreas Schwab 2017-07-26 14:26 ` Torsten Duwe 2017-07-26 14:33 ` Andreas Schwab 2017-07-31 11:47 ` Maxim Kuvyrkov 2017-08-01 10:43 ` Gerald Pfeifer 2017-08-01 10:53 ` Andreas Schwab 2017-08-01 13:03 ` Maxim Kuvyrkov 2017-08-01 13:36 ` Andreas Schwab 2017-08-11 12:42 ` [PATCH] skip patchable_function_entry tests on ia64 Torsten Duwe 2017-08-14 16:50 ` Jim Wilson
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).