[PATCH 0/3] Update -fpatchable-function-entry implementation

[PATCH 0/3] Update -fpatchable-function-entry implementation

[H.J. Lu]

The current -fpatchable-function-entry implementation is done almost
behind the backend back.  Backend doesn't know if and where patchable
area will be generated during RTL passes.

TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is only used to print out
assembly codes for patchable area.   This leads to wrong codes with
-fpatchable-function-entry:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93492

Also .cfi_startproc and DWARF info are at the wrong places when
-fpatchable-function-entry is used.

This patch set has 3 parts:

1. Add a pseudo UNSPECV_PATCHABLE_AREA instruction and define
TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to work around the
-fpatchable-function-entry implementation deficiency.
2. Add patch_area_size and patch_area_entry to cfun to make the 
patchable area info is available in RTL passes so that backend
can handle patchable area properly.  It also limits patch_area_size
and patch_area_entry to 65535, which is a reasonable maximum size
for patchable area.  Other backends can also use it properly generate
patchable area.
3. Remove workaround in UNSPECV_PATCHABLE_AREA generation. 

If the patch set is acceptable, I can combine patch 1 and patch 3
into a single patch.

H.J. Lu (3):
  x86: Add UNSPECV_PATCHABLE_AREA
  Add patch_area_size and patch_area_entry to cfun
  x86: Simplify UNSPECV_PATCHABLE_AREA generation

 gcc/config/i386/i386-features.c               | 130 ++++++++++++------
 gcc/config/i386/i386-passes.def               |   2 +-
 gcc/config/i386/i386-protos.h                 |   5 +-
 gcc/config/i386/i386.c                        |  51 ++++++-
 gcc/config/i386/i386.h                        |  14 +-
 gcc/config/i386/i386.md                       |  17 +++
 gcc/doc/invoke.texi                           |   1 +
 gcc/function.c                                |  35 +++++
 gcc/function.h                                |   6 +
 gcc/opts.c                                    |   4 +-
 .../patchable_function_entry-error-1.c        |   9 ++
 .../patchable_function_entry-error-2.c        |   9 ++
 .../patchable_function_entry-error-3.c        |  20 +++
 gcc/testsuite/gcc.target/i386/pr93492-1.c     |  73 ++++++++++
 gcc/testsuite/gcc.target/i386/pr93492-2.c     |  12 ++
 gcc/testsuite/gcc.target/i386/pr93492-3.c     |  13 ++
 gcc/testsuite/gcc.target/i386/pr93492-4.c     |  11 ++
 gcc/testsuite/gcc.target/i386/pr93492-5.c     |  12 ++
 gcc/varasm.c                                  |  30 +---
 19 files changed, 375 insertions(+), 79 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-5.c

-- 
2.24.1

[PATCH 1/3] x86: Add UNSPECV_PATCHABLE_AREA

[H.J. Lu]

Currently patchable area is at the wrong place.  It is placed immediately
after function label, before both .cfi_startproc and ENDBR.  This patch
adds UNSPECV_PATCHABLE_AREA for pseudo patchable area instruction and
changes ENDBR insertion pass to also insert a dummy patchable area.
TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY is defined to provide the
actual size for patchable area.  It places patchable area immediately
after .cfi_startproc and ENDBR.

gcc/

	PR target/93492
	* config/i386/i386-features.c (rest_of_insert_endbranch):
	Renamed to ...
	(rest_of_insert_endbr_and_patchable_area): Change return type
	to void.  Don't call timevar_push nor timevar_pop.  Replace
	endbr_queued_at_entrance with insn_queued_at_entrance.  Insert
	UNSPECV_PATCHABLE_AREA for patchable area.
	(pass_data_insert_endbranch): Renamed to ...
	(pass_data_insert_endbr_and_patchable_area): This.  Change
	pass name to endbr_and_patchable_area.
	(pass_insert_endbranch): Renamed to ...
	(pass_insert_endbr_and_patchable_area): This.  Add need_endbr
	and need_patchable_area.
	(pass_insert_endbr_and_patchable_area::gate): Set and check
	need_endbr/need_patchable_area.
	(pass_insert_endbr_and_patchable_area::execute): Call
	timevar_push and timevar_pop.  Pass need_endbr amd
	need_patchable_area to rest_of_insert_endbr_and_patchable_area.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386-passes.def: Replace pass_insert_endbranch
	with pass_insert_endbr_and_patchable_area.
	* config/i386/i386-protos.h (ix86_output_patchable_area): New.
	(make_pass_insert_endbranch): Renamed to ...
	(make_pass_insert_endbr_and_patchable_area): This.
	* config/i386/i386.c (ix86_asm_output_function_label): Set
	function_label_emitted to true.
	(ix86_print_patchable_function_entry): New function.
	(ix86_output_patchable_area): Likewise.
	(x86_function_profiler): Replace endbr_queued_at_entrance with
	insn_queued_at_entrance.  Generate ENDBR only for TYPE_ENDBR.
	Call ix86_output_patchable_area to generate patchable area.
	(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
	* i386.h (queued_insn_type): New.
	(machine_function): Add patch_area_size, record_patch_area and
	function_label_emitted.  Replace endbr_queued_at_entrance with
	insn_queued_at_entrance.
	* config/i386/i386.md (UNSPECV_PATCHABLE_AREA): New.
	(patchable_area): New.

gcc/testsuite/

	PR target/93492
	* gcc.target/i386/pr93492-1.c: New test.
	* gcc.target/i386/pr93492-2.c: Likewise.
	* gcc.target/i386/pr93492-3.c: Likewise.
	* gcc.target/i386/pr93492-4.c: Likewise.
	* gcc.target/i386/pr93492-5.c: Likewise.
---
 gcc/config/i386/i386-features.c           | 136 +++++++++++++++-------
 gcc/config/i386/i386-passes.def           |   2 +-
 gcc/config/i386/i386-protos.h             |   5 +-
 gcc/config/i386/i386.c                    |  91 ++++++++++++++-
 gcc/config/i386/i386.h                    |  20 +++-
 gcc/config/i386/i386.md                   |  15 +++
 gcc/testsuite/gcc.target/i386/pr93492-1.c |  73 ++++++++++++
 gcc/testsuite/gcc.target/i386/pr93492-2.c |  12 ++
 gcc/testsuite/gcc.target/i386/pr93492-3.c |  13 +++
 gcc/testsuite/gcc.target/i386/pr93492-4.c |  11 ++
 gcc/testsuite/gcc.target/i386/pr93492-5.c |  12 ++
 11 files changed, 339 insertions(+), 51 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-5.c

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index b49e6f8d408..be46f036126 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1937,43 +1937,79 @@ make_pass_stv (gcc::context *ctxt)
   return new pass_stv (ctxt);
 }
 
-/* Inserting ENDBRANCH instructions.  */
+/* Inserting ENDBR and pseudo patchable-area instructions.  */
 
-static unsigned int
-rest_of_insert_endbranch (void)
+static void
+rest_of_insert_endbr_and_patchable_area (bool need_endbr,
+					 bool need_patchable_area)
 {
-  timevar_push (TV_MACH_DEP);
-
-  rtx cet_eb;
+  rtx endbr;
   rtx_insn *insn;
+  rtx_insn *endbr_insn = NULL;
   basic_block bb;
 
-  /* Currently emit EB if it's a tracking function, i.e. 'nocf_check' is
-     absent among function attributes.  Later an optimization will be
-     introduced to make analysis if an address of a static function is
-     taken.  A static function whose address is not taken will get a
-     nocf_check attribute.  This will allow to reduce the number of EB.  */
+  if (need_endbr)
+    {
+      /* Currently emit EB if it's a tracking function, i.e. 'nocf_check'
+	 is absent among function attributes.  Later an optimization will
+	 be introduced to make analysis if an address of a static function
+	 is taken.  A static function whose address is not taken will get
+	 a nocf_check attribute.  This will allow to reduce the number of
+	 EB.  */
+      if (!lookup_attribute ("nocf_check",
+			     TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+	  && (!flag_manual_endbr
+	      || lookup_attribute ("cf_check",
+				   DECL_ATTRIBUTES (cfun->decl)))
+	  && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+	{
+	  if (crtl->profile && flag_fentry)
+	    {
+	      /* Queue ENDBR insertion to x86_function_profiler.
+		 NB: Any patchable-area insn will be inserted after
+		 ENDBR.  */
+	      cfun->machine->insn_queued_at_entrance = TYPE_ENDBR;
+	    }
+	  else
+	    {
+	      endbr = gen_nop_endbr ();
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      rtx_insn *insn = BB_HEAD (bb);
+	      endbr_insn = emit_insn_before (endbr, insn);
+	    }
+	}
+    }
 
-  if (!lookup_attribute ("nocf_check",
-			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
-      && (!flag_manual_endbr
-	  || lookup_attribute ("cf_check",
-			       DECL_ATTRIBUTES (cfun->decl)))
-      && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+  if (need_patchable_area)
     {
-      /* Queue ENDBR insertion to x86_function_profiler.  */
       if (crtl->profile && flag_fentry)
-	cfun->machine->endbr_queued_at_entrance = true;
+	{
+	  /* Queue patchable-area insertion to x86_function_profiler.
+	     NB: If there is a queued ENDBR, x86_function_profiler
+	     will also handle patchable-area.  */
+	  if (!cfun->machine->insn_queued_at_entrance)
+	    cfun->machine->insn_queued_at_entrance = TYPE_PATCHABLE_AREA;
+	}
       else
 	{
-	  cet_eb = gen_nop_endbr ();
-
-	  bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
-	  insn = BB_HEAD (bb);
-	  emit_insn_before (cet_eb, insn);
+	  /* ix86_print_patchable_function_entry will provide actual
+	     size.  */
+	  rtx patchable_area = gen_patchable_area (GEN_INT (0),
+						   GEN_INT (0));
+	  if (endbr_insn)
+	    emit_insn_after (patchable_area, endbr_insn);
+	  else
+	    {
+	      bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb;
+	      insn = BB_HEAD (bb);
+	      emit_insn_before (patchable_area, insn);
+	    }
 	}
     }
 
+  if (!need_endbr)
+    return;
+
   bb = 0;
   FOR_EACH_BB_FN (bb, cfun)
     {
@@ -1982,7 +2018,6 @@ rest_of_insert_endbranch (void)
 	{
 	  if (CALL_P (insn))
 	    {
-	      bool need_endbr;
 	      need_endbr = find_reg_note (insn, REG_SETJMP, NULL) != NULL;
 	      if (!need_endbr && !SIBLING_CALL_P (insn))
 		{
@@ -2013,8 +2048,8 @@ rest_of_insert_endbranch (void)
 	      /* Generate ENDBRANCH after CALL, which can return more than
 		 twice, setjmp-like functions.  */
 
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after_setloc (cet_eb, insn, INSN_LOCATION (insn));
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after_setloc (endbr, insn, INSN_LOCATION (insn));
 	      continue;
 	    }
 
@@ -2044,31 +2079,30 @@ rest_of_insert_endbranch (void)
 		  dest_blk = e->dest;
 		  insn = BB_HEAD (dest_blk);
 		  gcc_assert (LABEL_P (insn));
-		  cet_eb = gen_nop_endbr ();
-		  emit_insn_after (cet_eb, insn);
+		  endbr = gen_nop_endbr ();
+		  emit_insn_after (endbr, insn);
 		}
 	      continue;
 	    }
 
 	  if (LABEL_P (insn) && LABEL_PRESERVE_P (insn))
 	    {
-	      cet_eb = gen_nop_endbr ();
-	      emit_insn_after (cet_eb, insn);
+	      endbr = gen_nop_endbr ();
+	      emit_insn_after (endbr, insn);
 	      continue;
 	    }
 	}
     }
 
-  timevar_pop (TV_MACH_DEP);
-  return 0;
+  return;
 }
 
 namespace {
 
-const pass_data pass_data_insert_endbranch =
+const pass_data pass_data_insert_endbr_and_patchable_area =
 {
   RTL_PASS, /* type.  */
-  "cet", /* name.  */
+  "endbr_and_patchable_area", /* name.  */
   OPTGROUP_NONE, /* optinfo_flags.  */
   TV_MACH_DEP, /* tv_id.  */
   0, /* properties_required.  */
@@ -2078,32 +2112,44 @@ const pass_data pass_data_insert_endbranch =
   0, /* todo_flags_finish.  */
 };
 
-class pass_insert_endbranch : public rtl_opt_pass
+class pass_insert_endbr_and_patchable_area : public rtl_opt_pass
 {
 public:
-  pass_insert_endbranch (gcc::context *ctxt)
-    : rtl_opt_pass (pass_data_insert_endbranch, ctxt)
+  pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
+    : rtl_opt_pass (pass_data_insert_endbr_and_patchable_area, ctxt)
   {}
 
   /* opt_pass methods: */
-  virtual bool gate (function *)
-    {
-      return ((flag_cf_protection & CF_BRANCH));
+  virtual bool gate (function *fun)
+    {
+      need_endbr = (flag_cf_protection & CF_BRANCH) != 0;
+      need_patchable_area
+	= (function_entry_patch_area_size
+	   || lookup_attribute ("patchable_function_entry",
+				DECL_ATTRIBUTES (fun->decl)));
+      return need_endbr || need_patchable_area;
     }
 
   virtual unsigned int execute (function *)
     {
-      return rest_of_insert_endbranch ();
+      timevar_push (TV_MACH_DEP);
+      rest_of_insert_endbr_and_patchable_area (need_endbr,
+					       need_patchable_area);
+      timevar_pop (TV_MACH_DEP);
+      return 0;
     }
 
-}; // class pass_insert_endbranch
+private:
+  bool need_endbr;
+  bool need_patchable_area;
+}; // class pass_insert_endbr_and_patchable_area
 
 } // anon namespace
 
 rtl_opt_pass *
-make_pass_insert_endbranch (gcc::context *ctxt)
+make_pass_insert_endbr_and_patchable_area (gcc::context *ctxt)
 {
-  return new pass_insert_endbranch (ctxt);
+  return new pass_insert_endbr_and_patchable_area (ctxt);
 }
 
 /* At entry of the nearest common dominator for basic blocks with
diff --git a/gcc/config/i386/i386-passes.def b/gcc/config/i386/i386-passes.def
index 41386a13d88..d83c7b956b1 100644
--- a/gcc/config/i386/i386-passes.def
+++ b/gcc/config/i386/i386-passes.def
@@ -30,6 +30,6 @@ along with GCC; see the file COPYING3.  If not see
      CONSTM1_RTX generated by the STV pass can be CSEed.  */
   INSERT_PASS_BEFORE (pass_cse2, 1, pass_stv, true /* timode_p */);
 
-  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbranch);
+  INSERT_PASS_BEFORE (pass_shorten_branches, 1, pass_insert_endbr_and_patchable_area);
 
   INSERT_PASS_AFTER (pass_combine, 1, pass_remove_partial_avx_dependency);
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 266381ca5a6..cbcc08ebfe8 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -87,6 +87,8 @@ extern const char *output_fp_compare (rtx_insn *, rtx*, bool, bool);
 extern const char *output_adjust_stack_and_probe (rtx);
 extern const char *output_probe_stack_range (rtx, rtx);
 
+extern void ix86_output_patchable_area (unsigned int, bool);
+
 extern void ix86_expand_clear (rtx);
 extern void ix86_expand_move (machine_mode, rtx[]);
 extern void ix86_expand_vector_move (machine_mode, rtx[]);
@@ -376,6 +378,7 @@ class rtl_opt_pass;
 
 extern rtl_opt_pass *make_pass_insert_vzeroupper (gcc::context *);
 extern rtl_opt_pass *make_pass_stv (gcc::context *);
-extern rtl_opt_pass *make_pass_insert_endbranch (gcc::context *);
+extern rtl_opt_pass *make_pass_insert_endbr_and_patchable_area
+  (gcc::context *);
 extern rtl_opt_pass *make_pass_remove_partial_avx_dependency
   (gcc::context *);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffda3e8fd21..051a1fcbdc2 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1563,6 +1563,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
 {
   bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
 
+  if (cfun)
+    cfun->machine->function_label_emitted = true;
+
   if (is_ms_hook)
     {
       int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9118,6 +9121,80 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
     }
 }
 
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY.  */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+				     unsigned HOST_WIDE_INT patch_area_size,
+				     bool record_p)
+{
+  if (cfun->machine->function_label_emitted)
+    {
+      /* The insert_endbr_and_patchable_area pass inserted a dummy
+	 UNSPECV_PATCHABLE_AREA with 0 patchable area size.  If the
+	 patchable area is placed after the function label, we replace
+	 0 patchable area size with the real one.  Otherwise, the
+	 dummy UNSPECV_PATCHABLE_AREA will be ignored.  */
+      if (cfun->machine->insn_queued_at_entrance)
+	{
+	  /* Record the patchable area.  Both ENDBR and patchable area
+	     will be inserted by x86_function_profiler later.  */
+	  cfun->machine->patch_area_size = patch_area_size;
+	  cfun->machine->record_patch_area = record_p;
+	  return;
+	}
+
+      /* We can have
+
+	 UNSPECV_NOP_ENDBR
+	 UNSPECV_PATCHABLE_AREA
+
+	 or just
+
+	 UNSPECV_PATCHABLE_AREA
+       */
+      rtx_insn *patchable_insn;
+      rtx_insn *insn = next_real_nondebug_insn (get_insns ());
+      if (insn
+	  && INSN_P (insn)
+	  && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
+	  && XINT (PATTERN (insn), 1) == UNSPECV_NOP_ENDBR)
+	patchable_insn = next_real_nondebug_insn (insn);
+      else
+	patchable_insn = insn;
+
+      if (patchable_insn && INSN_P (patchable_insn))
+	{
+	  /* Replace the dummy patchable area size with the real one.  */
+	  rtx pattern = PATTERN (patchable_insn);
+	  if (GET_CODE (pattern) == UNSPEC_VOLATILE
+	      && XINT (pattern, 1) == UNSPECV_PATCHABLE_AREA)
+	    {
+	      XVECEXP (pattern, 0, 0) = GEN_INT (patch_area_size);
+	      XVECEXP (pattern, 0, 1) = GEN_INT (record_p);
+	    }
+	  return;
+	}
+
+      gcc_unreachable ();
+    }
+
+  default_print_patchable_function_entry (file, patch_area_size,
+					  record_p);
+}
+
+/* Output patchable area.  NB: default_print_patchable_function_entry
+   isn't available in i386.md.  */
+
+void
+ix86_output_patchable_area (unsigned int patch_area_size,
+			    bool record_p)
+{
+  default_print_patchable_function_entry (asm_out_file,
+					  patch_area_size,
+					  record_p);
+}
+
 /* Return a scratch register to use in the split stack prologue.  The
    split stack prologue is used for -fsplit-stack.  It is the first
    instructions in the function, even before the regular prologue.
@@ -20151,8 +20228,14 @@ current_fentry_section (const char **name)
 void
 x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
 {
-  if (cfun->machine->endbr_queued_at_entrance)
-    fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+  if (cfun->machine->insn_queued_at_entrance)
+    {
+      if (cfun->machine->insn_queued_at_entrance == TYPE_ENDBR)
+	fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+      if (cfun->machine->patch_area_size)
+	ix86_output_patchable_area (cfun->machine->patch_area_size,
+				    cfun->machine->record_patch_area);
+    }
 
   const char *mcount_name = MCOUNT_NAME;
 
@@ -22744,6 +22827,10 @@ ix86_run_selftests (void)
 #undef TARGET_ASM_FUNCTION_EPILOGUE
 #define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
 
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+  ix86_print_patchable_function_entry
+
 #undef TARGET_ENCODE_SECTION_INFO
 #ifndef SUBTARGET_ENCODE_SECTION_INFO
 #define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 943e9a5c783..7c8f0cd4c70 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2751,6 +2751,13 @@ enum function_type
   TYPE_EXCEPTION
 };
 
+enum queued_insn_type
+{
+  TYPE_NONE = 0,
+  TYPE_ENDBR,
+  TYPE_PATCHABLE_AREA
+};
+
 struct GTY(()) machine_function {
   struct stack_local_entry *stack_locals;
   int varargs_gpr_size;
@@ -2767,6 +2774,12 @@ struct GTY(()) machine_function {
      structure.  */
   rtx split_stack_varargs_pointer;
 
+  /* The size of the patchable area at function entry.  */
+  unsigned int patch_area_size;
+
+  /* If true, record patchable area at function entry.  */
+  BOOL_BITFIELD record_patch_area : 1;
+
   /* This value is used for amd64 targets and specifies the current abi
      to be used. MS_ABI means ms abi. Otherwise SYSV_ABI means sysv abi.  */
   ENUM_BITFIELD(calling_abi) call_abi : 8;
@@ -2841,8 +2854,11 @@ struct GTY(()) machine_function {
   /* Nonzero if the function places outgoing arguments on stack.  */
   BOOL_BITFIELD outgoing_args_on_stack : 1;
 
-  /* If true, ENDBR is queued at function entrance.  */
-  BOOL_BITFIELD endbr_queued_at_entrance : 1;
+  /* If true, ENDBR or patchable area is queued at function entrance.  */
+  ENUM_BITFIELD(queued_insn_type) insn_queued_at_entrance : 2;
+
+  /* If true, the function label has been emitted.  */
+  BOOL_BITFIELD function_label_emitted : 1;
 
   /* True if the function needs a stack frame.  */
   BOOL_BITFIELD stack_frame_required : 1;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 46b442dae51..78c38d5704a 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -300,6 +300,9 @@ (define_c_enum "unspecv" [
   ;; For ENQCMD and ENQCMDS support
   UNSPECV_ENQCMD
   UNSPECV_ENQCMDS
+
+  ;; For patchable area support
+  UNSPECV_PATCHABLE_AREA
 ])
 
 ;; Constants to represent rounding modes in the ROUND instruction
@@ -21280,6 +21283,18 @@ (define_insn "speculation_barrier"
   [(set_attr "type" "other")
    (set_attr "length" "3")])
 
+(define_insn "patchable_area"
+  [(unspec_volatile [(match_operand 0 "const_int_operand")
+		     (match_operand 1 "const_int_operand")]
+		    UNSPECV_PATCHABLE_AREA)]
+  ""
+{
+  if (INTVAL (operands[0]))
+    ix86_output_patchable_area (INTVAL (operands[0]),
+				INTVAL (operands[1]) != 0);
+  return "";
+})
+
 (include "mmx.md")
 (include "sse.md")
 (include "sync.md")
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-1.c b/gcc/testsuite/gcc.target/i386/pr93492-1.c
new file mode 100644
index 00000000000..f978d2e5220
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-1.c
@@ -0,0 +1,73 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr" } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/* Note: this test only checks the instructions in the function bodies,
+   not the placement of the patch label or nops before the function.  */
+
+/*
+**f10_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 0)))
+f10_none (void)
+{
+}
+
+/*
+**f10_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/*
+**f11_none:
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 1)))
+f11_none (void)
+{
+}
+
+/*
+**f11_endbr:
+**	endbr(32|64)
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 1)))
+f11_endbr (void)
+{
+}
+
+/*
+**f21_none:
+**	nop
+**	ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (2, 1)))
+f21_none (void)
+{
+}
+
+/*
+**f21_endbr:
+**	endbr(32|64)
+**	nop
+**	ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (2, 1)))
+f21_endbr (void)
+{
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-2.c b/gcc/testsuite/gcc.target/i386/pr93492-2.c
new file mode 100644
index 00000000000..ec26d4cc367
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-2.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-3.c b/gcc/testsuite/gcc.target/i386/pr93492-3.c
new file mode 100644
index 00000000000..1f03c627120
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-3.c
@@ -0,0 +1,13 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-4.c b/gcc/testsuite/gcc.target/i386/pr93492-4.c
new file mode 100644
index 00000000000..d193df8e66d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-4.c
@@ -0,0 +1,11 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-5.c b/gcc/testsuite/gcc.target/i386/pr93492-5.c
new file mode 100644
index 00000000000..d04077c6007
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-5.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fpatchable-function-entry=1 -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+foo (void)
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
-- 
2.24.1

[PATCH 3/3] x86: Simplify UNSPECV_PATCHABLE_AREA generation

[H.J. Lu]

Since patch_area_size and patch_area_entry have been added to cfun,
we can use them to directly insert the pseudo UNSPECV_PATCHABLE_AREA
instruction.

	PR target/93492
	* config/i386/i386-features.c
	(rest_of_insert_endbr_and_patchable_area): Change
	need_patchable_area argument to patchable_area_size.  Generate
	UNSPECV_PATCHABLE_AREA instruction with proper arguments.
	(pass_insert_endbr_and_patchable_area::gate): Set and check
	patchable_area_size instead of need_patchable_area.
	(pass_insert_endbr_and_patchable_area::execute): Pass
	patchable_area_size to rest_of_insert_endbr_and_patchable_area.
	(pass_insert_endbr_and_patchable_area): Replace
	need_patchable_area with patchable_area_size.
	* config/i386/i386.c (ix86_print_patchable_function_entry):
	Just return if function table has been emitted.
	(x86_function_profiler): Use cfun->patch_area_size and
	cfun->patch_area_entry.
	* config/i386/i386.h (machine_function): Remove patch_area_size
	and record_patch_area.
	* config/i386/i386.md (patchable_area): Set length attribute.
---
 gcc/config/i386/i386-features.c | 22 +++++-------
 gcc/config/i386/i386.c          | 60 ++++++---------------------------
 gcc/config/i386/i386.h          |  6 ----
 gcc/config/i386/i386.md         | 10 +++---
 4 files changed, 25 insertions(+), 73 deletions(-)

diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index be46f036126..d358abe7064 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1941,7 +1941,7 @@ make_pass_stv (gcc::context *ctxt)
 
 static void
 rest_of_insert_endbr_and_patchable_area (bool need_endbr,
-					 bool need_patchable_area)
+					 unsigned int patchable_area_size)
 {
   rtx endbr;
   rtx_insn *insn;
@@ -1980,7 +1980,7 @@ rest_of_insert_endbr_and_patchable_area (bool need_endbr,
 	}
     }
 
-  if (need_patchable_area)
+  if (patchable_area_size)
     {
       if (crtl->profile && flag_fentry)
 	{
@@ -1992,10 +1992,9 @@ rest_of_insert_endbr_and_patchable_area (bool need_endbr,
 	}
       else
 	{
-	  /* ix86_print_patchable_function_entry will provide actual
-	     size.  */
-	  rtx patchable_area = gen_patchable_area (GEN_INT (0),
-						   GEN_INT (0));
+	  rtx patchable_area
+	    = gen_patchable_area (GEN_INT (patchable_area_size),
+				  GEN_INT (cfun->patch_area_entry == 0));
 	  if (endbr_insn)
 	    emit_insn_after (patchable_area, endbr_insn);
 	  else
@@ -2123,25 +2122,22 @@ public:
   virtual bool gate (function *fun)
     {
       need_endbr = (flag_cf_protection & CF_BRANCH) != 0;
-      need_patchable_area
-	= (function_entry_patch_area_size
-	   || lookup_attribute ("patchable_function_entry",
-				DECL_ATTRIBUTES (fun->decl)));
-      return need_endbr || need_patchable_area;
+      patchable_area_size = fun->patch_area_size - fun->patch_area_entry;
+      return need_endbr || patchable_area_size;
     }
 
   virtual unsigned int execute (function *)
     {
       timevar_push (TV_MACH_DEP);
       rest_of_insert_endbr_and_patchable_area (need_endbr,
-					       need_patchable_area);
+					       patchable_area_size);
       timevar_pop (TV_MACH_DEP);
       return 0;
     }
 
 private:
   bool need_endbr;
-  bool need_patchable_area;
+  unsigned int patchable_area_size;
 }; // class pass_insert_endbr_and_patchable_area
 
 } // anon namespace
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 051a1fcbdc2..79a8823f61e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -9130,53 +9130,11 @@ ix86_print_patchable_function_entry (FILE *file,
 {
   if (cfun->machine->function_label_emitted)
     {
-      /* The insert_endbr_and_patchable_area pass inserted a dummy
-	 UNSPECV_PATCHABLE_AREA with 0 patchable area size.  If the
-	 patchable area is placed after the function label, we replace
-	 0 patchable area size with the real one.  Otherwise, the
-	 dummy UNSPECV_PATCHABLE_AREA will be ignored.  */
-      if (cfun->machine->insn_queued_at_entrance)
-	{
-	  /* Record the patchable area.  Both ENDBR and patchable area
-	     will be inserted by x86_function_profiler later.  */
-	  cfun->machine->patch_area_size = patch_area_size;
-	  cfun->machine->record_patch_area = record_p;
-	  return;
-	}
-
-      /* We can have
-
-	 UNSPECV_NOP_ENDBR
-	 UNSPECV_PATCHABLE_AREA
-
-	 or just
-
-	 UNSPECV_PATCHABLE_AREA
-       */
-      rtx_insn *patchable_insn;
-      rtx_insn *insn = next_real_nondebug_insn (get_insns ());
-      if (insn
-	  && INSN_P (insn)
-	  && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE
-	  && XINT (PATTERN (insn), 1) == UNSPECV_NOP_ENDBR)
-	patchable_insn = next_real_nondebug_insn (insn);
-      else
-	patchable_insn = insn;
-
-      if (patchable_insn && INSN_P (patchable_insn))
-	{
-	  /* Replace the dummy patchable area size with the real one.  */
-	  rtx pattern = PATTERN (patchable_insn);
-	  if (GET_CODE (pattern) == UNSPEC_VOLATILE
-	      && XINT (pattern, 1) == UNSPECV_PATCHABLE_AREA)
-	    {
-	      XVECEXP (pattern, 0, 0) = GEN_INT (patch_area_size);
-	      XVECEXP (pattern, 0, 1) = GEN_INT (record_p);
-	    }
-	  return;
-	}
-
-      gcc_unreachable ();
+      /* NB: When ix86_print_patchable_function_entry is called after
+	 function table has been emitted, we have inserted or queued
+	 a pseudo UNSPECV_PATCHABLE_AREA instruction at the proper
+	 place.  There is nothing to do here.  */
+      return;
     }
 
   default_print_patchable_function_entry (file, patch_area_size,
@@ -20232,9 +20190,11 @@ x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
     {
       if (cfun->machine->insn_queued_at_entrance == TYPE_ENDBR)
 	fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
-      if (cfun->machine->patch_area_size)
-	ix86_output_patchable_area (cfun->machine->patch_area_size,
-				    cfun->machine->record_patch_area);
+      unsigned int patch_area_size
+	= cfun->patch_area_size - cfun->patch_area_entry;
+      if (patch_area_size)
+	ix86_output_patchable_area (patch_area_size,
+				    cfun->patch_area_entry == 0);
     }
 
   const char *mcount_name = MCOUNT_NAME;
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 7c8f0cd4c70..e63edf16bcc 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2774,12 +2774,6 @@ struct GTY(()) machine_function {
      structure.  */
   rtx split_stack_varargs_pointer;
 
-  /* The size of the patchable area at function entry.  */
-  unsigned int patch_area_size;
-
-  /* If true, record patchable area at function entry.  */
-  BOOL_BITFIELD record_patch_area : 1;
-
   /* This value is used for amd64 targets and specifies the current abi
      to be used. MS_ABI means ms abi. Otherwise SYSV_ABI means sysv abi.  */
   ENUM_BITFIELD(calling_abi) call_abi : 8;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 78c38d5704a..3b243048d81 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -21289,11 +21289,13 @@ (define_insn "patchable_area"
 		    UNSPECV_PATCHABLE_AREA)]
   ""
 {
-  if (INTVAL (operands[0]))
-    ix86_output_patchable_area (INTVAL (operands[0]),
-				INTVAL (operands[1]) != 0);
+  ix86_output_patchable_area (INTVAL (operands[0]),
+			      INTVAL (operands[1]) != 0);
   return "";
-})
+}
+  [(set (attr "length") (symbol_ref "INTVAL (operands[0])"))
+   (set_attr "length_immediate" "0")
+   (set_attr "modrm" "0")])
 
 (include "mmx.md")
 (include "sse.md")
-- 
2.24.1

[PATCH 2/3] Add patch_area_size and patch_area_entry to cfun

[H.J. Lu]

Currently patchable area is at the wrong place.  It is placed immediately
after function label and before .cfi_startproc.  A backend should be able
to add a pseudo patchable area instruction durectly into RTL.  This patch
adds patch_area_size and patch_area_entry to cfun so that the patchable
area info is available in RTL passes.

It also limits patch_area_size and patch_area_entry to 65535, which is
a reasonable maximum size for patchable area.

gcc/

	PR target/93492
	* function.c (expand_function_start): Set cfun->patch_area_size
	and cfun->patch_area_entry.
	* function.h (function): Add patch_area_size and patch_area_entry.
	* opts.c (common_handle_option): Limit
	function_entry_patch_area_size and function_entry_patch_area_start
	to USHRT_MAX.  Fix a typo in error message.
	* varasm.c (assemble_start_function): Use cfun->patch_area_size
	and cfun->patch_area_entry.
	* doc/invoke.texi: Document the maximum value for
	-fpatchable-function-entry.

gcc/testsuite/

	PR target/93492
	* c-c++-common/patchable_function_entry-error-1.c: New test.
	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
	* c-c++-common/patchable_function_entry-error-3.c: Likewise.
---
 gcc/doc/invoke.texi                           |  1 +
 gcc/function.c                                | 35 +++++++++++++++++++
 gcc/function.h                                |  6 ++++
 gcc/opts.c                                    |  4 ++-
 .../patchable_function_entry-error-1.c        |  9 +++++
 .../patchable_function_entry-error-2.c        |  9 +++++
 .../patchable_function_entry-error-3.c        | 20 +++++++++++
 gcc/varasm.c                                  | 30 ++--------------
 8 files changed, 85 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 35b341e759f..dd4835199b0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
 The NOP instructions are inserted at---and maybe before, depending on
 @var{M}---the function entry address, even before the prologue.
 
+The maximum value of @var{N} and @var{M} is 65535.
 @end table
 
 
diff --git a/gcc/function.c b/gcc/function.c
index d8008f60422..badbf538eec 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
   /* If we are doing generic stack checking, the probe should go here.  */
   if (flag_stack_check == GENERIC_STACK_CHECK)
     stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
+
+  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+  tree patchable_function_entry_attr
+    = lookup_attribute ("patchable_function_entry",
+			DECL_ATTRIBUTES (cfun->decl));
+  if (patchable_function_entry_attr)
+    {
+      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+      patch_area_entry = 0;
+      if (TREE_CHAIN (pp_val) != NULL_TREE)
+	{
+	  tree patchable_function_entry_value2
+	    = TREE_VALUE (TREE_CHAIN (pp_val));
+	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+	}
+      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
+	error ("invalid values for %<patchable_function_entry%> attribute");
+    }
+
+  if (patch_area_entry > patch_area_size)
+    {
+      if (patch_area_size > 0)
+	warning (OPT_Wattributes,
+		 "patchable function entry %wu exceeds size %wu",
+		 patch_area_entry, patch_area_size);
+      patch_area_entry = 0;
+    }
+
+  cfun->patch_area_size = patch_area_size;
+  cfun->patch_area_entry = patch_area_entry;
 }
 \f
 void
diff --git a/gcc/function.h b/gcc/function.h
index 1ee8ed3de53..1ed7c400f23 100644
--- a/gcc/function.h
+++ b/gcc/function.h
@@ -332,6 +332,12 @@ struct GTY(()) function {
   /* Last assigned dependence info clique.  */
   unsigned short last_clique;
 
+  /* How many NOP insns to place at each function entry by default.  */
+  unsigned short patch_area_size;
+
+  /* How far the real asm entry point is into this area.  */
+  unsigned short patch_area_entry;
+
   /* Collected bit flags.  */
 
   /* Number of units of general registers that need saving in stdarg
diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb41a96..c6011f1f9b7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2598,10 +2598,12 @@ common_handle_option (struct gcc_options *opts,
 	    function_entry_patch_area_start = 0;
 	  }
 	if (function_entry_patch_area_size < 0
+	    || function_entry_patch_area_size > USHRT_MAX
 	    || function_entry_patch_area_start < 0
+	    || function_entry_patch_area_start > USHRT_MAX
 	    || function_entry_patch_area_size 
 		< function_entry_patch_area_start)
-	  error ("invalid arguments for %<-fpatchable_function_entry%>");
+	  error ("invalid arguments for %<-fpatchable-function-entry%>");
 	free (patch_area_arg);
       }
       break;
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
new file mode 100644
index 00000000000..f60bf46cfe3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=65536,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
new file mode 100644
index 00000000000..90f88c78be7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=1,65536" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
new file mode 100644
index 00000000000..b6e449425d2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+
+void
+ __attribute__((patchable_function_entry(65536)))
+foo1 (void) /* { dg-error "invalid values for 'patchable_function_entry'" } */
+{
+}
+
+void
+ __attribute__((patchable_function_entry(65536,1)))
+foo2 (void) /* { dg-error "invalid values for 'patchable_function_entry'" } */
+{
+}
+
+void
+ __attribute__((patchable_function_entry(65536,65536)))
+foo3 (void) /* { dg-error "invalid values for 'patchable_function_entry'" } */
+{
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index dc6da6c0b5b..93251f07d6e 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1857,34 +1857,8 @@ 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);
-
-      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
-      patch_area_entry = 0;
-      if (TREE_CHAIN (pp_val) != NULL_TREE)
-	{
-	  tree patchable_function_entry_value2
-	    = TREE_VALUE (TREE_CHAIN (pp_val));
-	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
-	}
-    }
-
-  if (patch_area_entry > patch_area_size)
-    {
-      if (patch_area_size > 0)
-	warning (OPT_Wattributes,
-		 "patchable function entry %wu exceeds size %wu",
-		 patch_area_entry, patch_area_size);
-      patch_area_entry = 0;
-    }
+  unsigned short patch_area_size = cfun->patch_area_size;
+  unsigned short patch_area_entry = cfun->patch_area_entry;
 
   /* Emit the patching area before the entry label, if any.  */
   if (patch_area_entry > 0)
-- 
2.24.1

Re: [PATCH 2/3] Add patch_area_size and patch_area_entry to cfun

[Richard Sandiford]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> Currently patchable area is at the wrong place.

Agreed :-)

> It is placed immediately
> after function label and before .cfi_startproc.  A backend should be able
> to add a pseudo patchable area instruction durectly into RTL.  This patch
> adds patch_area_size and patch_area_entry to cfun so that the patchable
> area info is available in RTL passes.

It might be better to add it to crtl, since it should only be needed
during rtl generation.

> It also limits patch_area_size and patch_area_entry to 65535, which is
> a reasonable maximum size for patchable area.
>
> gcc/
>
> 	PR target/93492
> 	* function.c (expand_function_start): Set cfun->patch_area_size
> 	and cfun->patch_area_entry.
> 	* function.h (function): Add patch_area_size and patch_area_entry.
> 	* opts.c (common_handle_option): Limit
> 	function_entry_patch_area_size and function_entry_patch_area_start
> 	to USHRT_MAX.  Fix a typo in error message.
> 	* varasm.c (assemble_start_function): Use cfun->patch_area_size
> 	and cfun->patch_area_entry.
> 	* doc/invoke.texi: Document the maximum value for
> 	-fpatchable-function-entry.
>
> gcc/testsuite/
>
> 	PR target/93492
> 	* c-c++-common/patchable_function_entry-error-1.c: New test.
> 	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
> 	* c-c++-common/patchable_function_entry-error-3.c: Likewise.
> ---
>  gcc/doc/invoke.texi                           |  1 +
>  gcc/function.c                                | 35 +++++++++++++++++++
>  gcc/function.h                                |  6 ++++
>  gcc/opts.c                                    |  4 ++-
>  .../patchable_function_entry-error-1.c        |  9 +++++
>  .../patchable_function_entry-error-2.c        |  9 +++++
>  .../patchable_function_entry-error-3.c        | 20 +++++++++++
>  gcc/varasm.c                                  | 30 ++--------------
>  8 files changed, 85 insertions(+), 29 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
>  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 35b341e759f..dd4835199b0 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
>  The NOP instructions are inserted at---and maybe before, depending on
>  @var{M}---the function entry address, even before the prologue.
>  
> +The maximum value of @var{N} and @var{M} is 65535.
>  @end table
>  
>  
> diff --git a/gcc/function.c b/gcc/function.c
> index d8008f60422..badbf538eec 100644
> --- a/gcc/function.c
> +++ b/gcc/function.c
> @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
>    /* If we are doing generic stack checking, the probe should go here.  */
>    if (flag_stack_check == GENERIC_STACK_CHECK)
>      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> +
> +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> +
> +  tree patchable_function_entry_attr
> +    = lookup_attribute ("patchable_function_entry",
> +			DECL_ATTRIBUTES (cfun->decl));
> +  if (patchable_function_entry_attr)
> +    {
> +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> +
> +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> +      patch_area_entry = 0;
> +      if (TREE_CHAIN (pp_val) != NULL_TREE)
> +	{
> +	  tree patchable_function_entry_value2
> +	    = TREE_VALUE (TREE_CHAIN (pp_val));
> +	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> +	}
> +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> +	error ("invalid values for %<patchable_function_entry%> attribute");

This should probably go in handle_patchable_function_entry_attribute
instead.  It doesn't look like we have a clear policy about whether
errors or warnings are right for unrecognised arguments to known
attribute names, but handle_patchable_function_entry_attribute reports
an OPT_Wattributes warning for arguments that aren't integers.  Doing the
same for out-of-range integers would be more consistent and means that
we wouldn't break existing code if we relaxed/changed the rules in future.

Thanks,
Richard

[PATCH] Add patch_area_size and patch_area_entry to crtl

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 4945 bytes --]

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

Like this?  OK for master if there is no regression?

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Add-patch_area_size-and-patch_area_entry-to-crtl.patch --]
[-- Type: text/x-patch, Size: 9775 bytes --]

From 8a56c3424d4194dfc0290eaa666962c6e75f9ce8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Feb 2020 04:01:59 -0800
Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl

Currently patchable area is at the wrong place.  It is placed immediately
after function label and before .cfi_startproc.  A backend should be able
to add a pseudo patchable area instruction durectly into RTL.  This patch
adds patch_area_size and patch_area_entry to crtl so that the patchable
area info is available in RTL passes.

It also limits patch_area_size and patch_area_entry to 65535, which is
a reasonable maximum size for patchable area.

gcc/c-family/

	PR target/93492
	* c-attribs.c (handle_patchable_function_entry_attribute): Limit
	value to USHRT_MAX (65535).

gcc/

	PR target/93492
	* cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size
	and crtl->patch_area_entry.
	* emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry.
	* opts.c (common_handle_option): Limit
	function_entry_patch_area_size and function_entry_patch_area_start
	to USHRT_MAX.  Fix a typo in error message.
	* varasm.c (assemble_start_function): Use crtl->patch_area_size
	and crtl->patch_area_entry.
	* doc/invoke.texi: Document the maximum value for
	-fpatchable-function-entry.

gcc/testsuite/

	PR target/93492
	* c-c++-common/patchable_function_entry-error-1.c: New test.
	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
	* c-c++-common/patchable_function_entry-error-3.c: Likewise.
---
 gcc/c-family/c-attribs.c                      |  9 +++++
 gcc/cfgexpand.c                               | 33 +++++++++++++++++++
 gcc/doc/invoke.texi                           |  1 +
 gcc/emit-rtl.h                                |  6 ++++
 gcc/opts.c                                    |  4 ++-
 .../patchable_function_entry-error-1.c        |  9 +++++
 .../patchable_function_entry-error-2.c        |  9 +++++
 .../patchable_function_entry-error-3.c        | 20 +++++++++++
 gcc/varasm.c                                  | 30 ++---------------
 9 files changed, 92 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 7ec6fc848ac..15dbda1eff7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4539,6 +4539,15 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
 	  *no_add_attrs = true;
 	  return NULL_TREE;
 	}
+
+      if (tree_to_uhwi (val) > USHRT_MAX)
+	{
+	  warning (OPT_Wattributes,
+		   "%qE attribute argument %qE is out of range (> 65535)",
+		   name, val);
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
     }
   return NULL_TREE;
 }
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 9864e4344d2..f063f50c263 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -6656,6 +6656,39 @@ pass_expand::execute (function *fun)
   if (crtl->tail_call_emit)
     fixup_tail_calls ();
 
+  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+  tree patchable_function_entry_attr
+    = lookup_attribute ("patchable_function_entry",
+			DECL_ATTRIBUTES (cfun->decl));
+  if (patchable_function_entry_attr)
+    {
+      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+      patch_area_entry = 0;
+      if (TREE_CHAIN (pp_val) != NULL_TREE)
+	{
+	  tree patchable_function_entry_value2
+	    = TREE_VALUE (TREE_CHAIN (pp_val));
+	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+	}
+    }
+
+  if (patch_area_entry > patch_area_size)
+    {
+      if (patch_area_size > 0)
+	warning (OPT_Wattributes,
+		 "patchable function entry %wu exceeds size %wu",
+		 patch_area_entry, patch_area_size);
+      patch_area_entry = 0;
+    }
+
+  crtl->patch_area_size = patch_area_size;
+  crtl->patch_area_entry = patch_area_entry;
+
   /* BB subdivision may have created basic blocks that are are only reachable
      from unlikely bbs but not marked as such in the profile.  */
   if (optimize)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 35b341e759f..dd4835199b0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
 The NOP instructions are inserted at---and maybe before, depending on
 @var{M}---the function entry address, even before the prologue.
 
+The maximum value of @var{N} and @var{M} is 65535.
 @end table
 
 
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index a878efe3cf7..3d6565c8a30 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -173,6 +173,12 @@ struct GTY(()) rtl_data {
         local stack.  */
   unsigned int stack_alignment_estimated;
 
+  /* How many NOP insns to place at each function entry by default.  */
+  unsigned short patch_area_size;
+
+  /* How far the real asm entry point is into this area.  */
+  unsigned short patch_area_entry;
+
   /* For reorg.  */
 
   /* Nonzero if function being compiled called builtin_return_addr or
diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb41a96..c6011f1f9b7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2598,10 +2598,12 @@ common_handle_option (struct gcc_options *opts,
 	    function_entry_patch_area_start = 0;
 	  }
 	if (function_entry_patch_area_size < 0
+	    || function_entry_patch_area_size > USHRT_MAX
 	    || function_entry_patch_area_start < 0
+	    || function_entry_patch_area_start > USHRT_MAX
 	    || function_entry_patch_area_size 
 		< function_entry_patch_area_start)
-	  error ("invalid arguments for %<-fpatchable_function_entry%>");
+	  error ("invalid arguments for %<-fpatchable-function-entry%>");
 	free (patch_area_arg);
       }
       break;
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
new file mode 100644
index 00000000000..f60bf46cfe3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=65536,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
new file mode 100644
index 00000000000..90f88c78be7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=1,65536" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
new file mode 100644
index 00000000000..45e93988886
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+
+void
+ __attribute__((patchable_function_entry(65536)))
+foo1 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,1)))
+foo2 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,65536)))
+foo3 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index dc6da6c0b5b..9179fecdf85 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1857,34 +1857,8 @@ 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);
-
-      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
-      patch_area_entry = 0;
-      if (TREE_CHAIN (pp_val) != NULL_TREE)
-	{
-	  tree patchable_function_entry_value2
-	    = TREE_VALUE (TREE_CHAIN (pp_val));
-	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
-	}
-    }
-
-  if (patch_area_entry > patch_area_size)
-    {
-      if (patch_area_size > 0)
-	warning (OPT_Wattributes,
-		 "patchable function entry %wu exceeds size %wu",
-		 patch_area_entry, patch_area_size);
-      patch_area_entry = 0;
-    }
+  unsigned short patch_area_size = crtl->patch_area_size;
+  unsigned short patch_area_entry = crtl->patch_area_entry;
 
   /* Emit the patching area before the entry label, if any.  */
   if (patch_area_entry > 0)
-- 
2.24.1

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[H.J. Lu]

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

There is a small problem.  Warnings from C and C++ frond-ends are different:

[hjl@gnu-skx-1 gcc]$ cat x.c
void
 __attribute__((patchable_function_entry(65536)))
foo1 (void)
{ /* { dg-warning "'patchable_function_entry' attribute argument
'65536' is out of range" } */
}
[hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
‘65536’ is out of range (> 65535) [-Wattributes]
    4 | { /* { dg-warning "'patchable_function_entry' attribute
argument '65536' is out of range" } */
      | ^
[hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
‘65536’ is out of range (> 65535) [-Wattributes]
    3 | foo1 (void)
      |           ^
[hjl@gnu-skx-1 gcc]$

C warns at line 4 and C++ warns at line 3.   Do I need separate tests
for C and C++?

-- 
H.J.

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[Marek Polacek]

On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote:
> On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford
> > <richard.sandiford@arm.com> wrote:
> > >
> > > "H.J. Lu" <hjl.tools@gmail.com> writes:
> > > > Currently patchable area is at the wrong place.
> > >
> > > Agreed :-)
> > >
> > > > It is placed immediately
> > > > after function label and before .cfi_startproc.  A backend should be able
> > > > to add a pseudo patchable area instruction durectly into RTL.  This patch
> > > > adds patch_area_size and patch_area_entry to cfun so that the patchable
> > > > area info is available in RTL passes.
> > >
> > > It might be better to add it to crtl, since it should only be needed
> > > during rtl generation.
> > >
> > > > It also limits patch_area_size and patch_area_entry to 65535, which is
> > > > a reasonable maximum size for patchable area.
> > > >
> > > > gcc/
> > > >
> > > >       PR target/93492
> > > >       * function.c (expand_function_start): Set cfun->patch_area_size
> > > >       and cfun->patch_area_entry.
> > > >       * function.h (function): Add patch_area_size and patch_area_entry.
> > > >       * opts.c (common_handle_option): Limit
> > > >       function_entry_patch_area_size and function_entry_patch_area_start
> > > >       to USHRT_MAX.  Fix a typo in error message.
> > > >       * varasm.c (assemble_start_function): Use cfun->patch_area_size
> > > >       and cfun->patch_area_entry.
> > > >       * doc/invoke.texi: Document the maximum value for
> > > >       -fpatchable-function-entry.
> > > >
> > > > gcc/testsuite/
> > > >
> > > >       PR target/93492
> > > >       * c-c++-common/patchable_function_entry-error-1.c: New test.
> > > >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> > > >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
> > > > ---
> > > >  gcc/doc/invoke.texi                           |  1 +
> > > >  gcc/function.c                                | 35 +++++++++++++++++++
> > > >  gcc/function.h                                |  6 ++++
> > > >  gcc/opts.c                                    |  4 ++-
> > > >  .../patchable_function_entry-error-1.c        |  9 +++++
> > > >  .../patchable_function_entry-error-2.c        |  9 +++++
> > > >  .../patchable_function_entry-error-3.c        | 20 +++++++++++
> > > >  gcc/varasm.c                                  | 30 ++--------------
> > > >  8 files changed, 85 insertions(+), 29 deletions(-)
> > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
> > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
> > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
> > > >
> > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > index 35b341e759f..dd4835199b0 100644
> > > > --- a/gcc/doc/invoke.texi
> > > > +++ b/gcc/doc/invoke.texi
> > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
> > > >  The NOP instructions are inserted at---and maybe before, depending on
> > > >  @var{M}---the function entry address, even before the prologue.
> > > >
> > > > +The maximum value of @var{N} and @var{M} is 65535.
> > > >  @end table
> > > >
> > > >
> > > > diff --git a/gcc/function.c b/gcc/function.c
> > > > index d8008f60422..badbf538eec 100644
> > > > --- a/gcc/function.c
> > > > +++ b/gcc/function.c
> > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
> > > >    /* If we are doing generic stack checking, the probe should go here.  */
> > > >    if (flag_stack_check == GENERIC_STACK_CHECK)
> > > >      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> > > > +
> > > > +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> > > > +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> > > > +
> > > > +  tree patchable_function_entry_attr
> > > > +    = lookup_attribute ("patchable_function_entry",
> > > > +                     DECL_ATTRIBUTES (cfun->decl));
> > > > +  if (patchable_function_entry_attr)
> > > > +    {
> > > > +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> > > > +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> > > > +
> > > > +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> > > > +      patch_area_entry = 0;
> > > > +      if (TREE_CHAIN (pp_val) != NULL_TREE)
> > > > +     {
> > > > +       tree patchable_function_entry_value2
> > > > +         = TREE_VALUE (TREE_CHAIN (pp_val));
> > > > +       patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> > > > +     }
> > > > +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> > > > +     error ("invalid values for %<patchable_function_entry%> attribute");
> > >
> > > This should probably go in handle_patchable_function_entry_attribute
> > > instead.  It doesn't look like we have a clear policy about whether
> > > errors or warnings are right for unrecognised arguments to known
> > > attribute names, but handle_patchable_function_entry_attribute reports
> > > an OPT_Wattributes warning for arguments that aren't integers.  Doing the
> > > same for out-of-range integers would be more consistent and means that
> > > we wouldn't break existing code if we relaxed/changed the rules in future.
> >
> > Like this?  OK for master if there is no regression?
> >
> 
> There is a small problem.  Warnings from C and C++ frond-ends are different:
> 
> [hjl@gnu-skx-1 gcc]$ cat x.c
> void
>  __attribute__((patchable_function_entry(65536)))
> foo1 (void)
> { /* { dg-warning "'patchable_function_entry' attribute argument
> '65536' is out of range" } */
> }
> [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
> x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
> ‘65536’ is out of range (> 65535) [-Wattributes]
>     4 | { /* { dg-warning "'patchable_function_entry' attribute
> argument '65536' is out of range" } */
>       | ^
> [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
> x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
> ‘65536’ is out of range (> 65535) [-Wattributes]
>     3 | foo1 (void)
>       |           ^
> [hjl@gnu-skx-1 gcc]$
> 
> C warns at line 4 and C++ warns at line 3.   Do I need separate tests
> for C and C++?

I think better would be

/* { dg-error "foo" "" { target c } } */
/* { dg-error "bar" "" { target c++ } .-1 } */

Marek

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[H.J. Lu]

On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote:
>
> On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote:
> > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford
> > > <richard.sandiford@arm.com> wrote:
> > > >
> > > > "H.J. Lu" <hjl.tools@gmail.com> writes:
> > > > > Currently patchable area is at the wrong place.
> > > >
> > > > Agreed :-)
> > > >
> > > > > It is placed immediately
> > > > > after function label and before .cfi_startproc.  A backend should be able
> > > > > to add a pseudo patchable area instruction durectly into RTL.  This patch
> > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable
> > > > > area info is available in RTL passes.
> > > >
> > > > It might be better to add it to crtl, since it should only be needed
> > > > during rtl generation.
> > > >
> > > > > It also limits patch_area_size and patch_area_entry to 65535, which is
> > > > > a reasonable maximum size for patchable area.
> > > > >
> > > > > gcc/
> > > > >
> > > > >       PR target/93492
> > > > >       * function.c (expand_function_start): Set cfun->patch_area_size
> > > > >       and cfun->patch_area_entry.
> > > > >       * function.h (function): Add patch_area_size and patch_area_entry.
> > > > >       * opts.c (common_handle_option): Limit
> > > > >       function_entry_patch_area_size and function_entry_patch_area_start
> > > > >       to USHRT_MAX.  Fix a typo in error message.
> > > > >       * varasm.c (assemble_start_function): Use cfun->patch_area_size
> > > > >       and cfun->patch_area_entry.
> > > > >       * doc/invoke.texi: Document the maximum value for
> > > > >       -fpatchable-function-entry.
> > > > >
> > > > > gcc/testsuite/
> > > > >
> > > > >       PR target/93492
> > > > >       * c-c++-common/patchable_function_entry-error-1.c: New test.
> > > > >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> > > > >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
> > > > > ---
> > > > >  gcc/doc/invoke.texi                           |  1 +
> > > > >  gcc/function.c                                | 35 +++++++++++++++++++
> > > > >  gcc/function.h                                |  6 ++++
> > > > >  gcc/opts.c                                    |  4 ++-
> > > > >  .../patchable_function_entry-error-1.c        |  9 +++++
> > > > >  .../patchable_function_entry-error-2.c        |  9 +++++
> > > > >  .../patchable_function_entry-error-3.c        | 20 +++++++++++
> > > > >  gcc/varasm.c                                  | 30 ++--------------
> > > > >  8 files changed, 85 insertions(+), 29 deletions(-)
> > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
> > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
> > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
> > > > >
> > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > index 35b341e759f..dd4835199b0 100644
> > > > > --- a/gcc/doc/invoke.texi
> > > > > +++ b/gcc/doc/invoke.texi
> > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
> > > > >  The NOP instructions are inserted at---and maybe before, depending on
> > > > >  @var{M}---the function entry address, even before the prologue.
> > > > >
> > > > > +The maximum value of @var{N} and @var{M} is 65535.
> > > > >  @end table
> > > > >
> > > > >
> > > > > diff --git a/gcc/function.c b/gcc/function.c
> > > > > index d8008f60422..badbf538eec 100644
> > > > > --- a/gcc/function.c
> > > > > +++ b/gcc/function.c
> > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
> > > > >    /* If we are doing generic stack checking, the probe should go here.  */
> > > > >    if (flag_stack_check == GENERIC_STACK_CHECK)
> > > > >      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> > > > > +
> > > > > +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> > > > > +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> > > > > +
> > > > > +  tree patchable_function_entry_attr
> > > > > +    = lookup_attribute ("patchable_function_entry",
> > > > > +                     DECL_ATTRIBUTES (cfun->decl));
> > > > > +  if (patchable_function_entry_attr)
> > > > > +    {
> > > > > +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> > > > > +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> > > > > +
> > > > > +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> > > > > +      patch_area_entry = 0;
> > > > > +      if (TREE_CHAIN (pp_val) != NULL_TREE)
> > > > > +     {
> > > > > +       tree patchable_function_entry_value2
> > > > > +         = TREE_VALUE (TREE_CHAIN (pp_val));
> > > > > +       patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> > > > > +     }
> > > > > +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> > > > > +     error ("invalid values for %<patchable_function_entry%> attribute");
> > > >
> > > > This should probably go in handle_patchable_function_entry_attribute
> > > > instead.  It doesn't look like we have a clear policy about whether
> > > > errors or warnings are right for unrecognised arguments to known
> > > > attribute names, but handle_patchable_function_entry_attribute reports
> > > > an OPT_Wattributes warning for arguments that aren't integers.  Doing the
> > > > same for out-of-range integers would be more consistent and means that
> > > > we wouldn't break existing code if we relaxed/changed the rules in future.
> > >
> > > Like this?  OK for master if there is no regression?
> > >
> >
> > There is a small problem.  Warnings from C and C++ frond-ends are different:
> >
> > [hjl@gnu-skx-1 gcc]$ cat x.c
> > void
> >  __attribute__((patchable_function_entry(65536)))
> > foo1 (void)
> > { /* { dg-warning "'patchable_function_entry' attribute argument
> > '65536' is out of range" } */
> > }
> > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
> > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
> > ‘65536’ is out of range (> 65535) [-Wattributes]
> >     4 | { /* { dg-warning "'patchable_function_entry' attribute
> > argument '65536' is out of range" } */
> >       | ^
> > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
> > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
> > ‘65536’ is out of range (> 65535) [-Wattributes]
> >     3 | foo1 (void)
> >       |           ^
> > [hjl@gnu-skx-1 gcc]$
> >
> > C warns at line 4 and C++ warns at line 3.   Do I need separate tests
> > for C and C++?
>
> I think better would be
>
> /* { dg-error "foo" "" { target c } } */
> /* { dg-error "bar" "" { target c++ } .-1 } */
>
> Marek
>

It worked.

Thanks.

-- 
H.J.

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 7518 bytes --]

On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote:
> >
> > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote:
> > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford
> > > > <richard.sandiford@arm.com> wrote:
> > > > >
> > > > > "H.J. Lu" <hjl.tools@gmail.com> writes:
> > > > > > Currently patchable area is at the wrong place.
> > > > >
> > > > > Agreed :-)
> > > > >
> > > > > > It is placed immediately
> > > > > > after function label and before .cfi_startproc.  A backend should be able
> > > > > > to add a pseudo patchable area instruction durectly into RTL.  This patch
> > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable
> > > > > > area info is available in RTL passes.
> > > > >
> > > > > It might be better to add it to crtl, since it should only be needed
> > > > > during rtl generation.
> > > > >
> > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is
> > > > > > a reasonable maximum size for patchable area.
> > > > > >
> > > > > > gcc/
> > > > > >
> > > > > >       PR target/93492
> > > > > >       * function.c (expand_function_start): Set cfun->patch_area_size
> > > > > >       and cfun->patch_area_entry.
> > > > > >       * function.h (function): Add patch_area_size and patch_area_entry.
> > > > > >       * opts.c (common_handle_option): Limit
> > > > > >       function_entry_patch_area_size and function_entry_patch_area_start
> > > > > >       to USHRT_MAX.  Fix a typo in error message.
> > > > > >       * varasm.c (assemble_start_function): Use cfun->patch_area_size
> > > > > >       and cfun->patch_area_entry.
> > > > > >       * doc/invoke.texi: Document the maximum value for
> > > > > >       -fpatchable-function-entry.
> > > > > >
> > > > > > gcc/testsuite/
> > > > > >
> > > > > >       PR target/93492
> > > > > >       * c-c++-common/patchable_function_entry-error-1.c: New test.
> > > > > >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> > > > > >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
> > > > > > ---
> > > > > >  gcc/doc/invoke.texi                           |  1 +
> > > > > >  gcc/function.c                                | 35 +++++++++++++++++++
> > > > > >  gcc/function.h                                |  6 ++++
> > > > > >  gcc/opts.c                                    |  4 ++-
> > > > > >  .../patchable_function_entry-error-1.c        |  9 +++++
> > > > > >  .../patchable_function_entry-error-2.c        |  9 +++++
> > > > > >  .../patchable_function_entry-error-3.c        | 20 +++++++++++
> > > > > >  gcc/varasm.c                                  | 30 ++--------------
> > > > > >  8 files changed, 85 insertions(+), 29 deletions(-)
> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
> > > > > >
> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > > > > index 35b341e759f..dd4835199b0 100644
> > > > > > --- a/gcc/doc/invoke.texi
> > > > > > +++ b/gcc/doc/invoke.texi
> > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
> > > > > >  The NOP instructions are inserted at---and maybe before, depending on
> > > > > >  @var{M}---the function entry address, even before the prologue.
> > > > > >
> > > > > > +The maximum value of @var{N} and @var{M} is 65535.
> > > > > >  @end table
> > > > > >
> > > > > >
> > > > > > diff --git a/gcc/function.c b/gcc/function.c
> > > > > > index d8008f60422..badbf538eec 100644
> > > > > > --- a/gcc/function.c
> > > > > > +++ b/gcc/function.c
> > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
> > > > > >    /* If we are doing generic stack checking, the probe should go here.  */
> > > > > >    if (flag_stack_check == GENERIC_STACK_CHECK)
> > > > > >      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> > > > > > +
> > > > > > +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> > > > > > +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> > > > > > +
> > > > > > +  tree patchable_function_entry_attr
> > > > > > +    = lookup_attribute ("patchable_function_entry",
> > > > > > +                     DECL_ATTRIBUTES (cfun->decl));
> > > > > > +  if (patchable_function_entry_attr)
> > > > > > +    {
> > > > > > +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> > > > > > +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> > > > > > +
> > > > > > +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> > > > > > +      patch_area_entry = 0;
> > > > > > +      if (TREE_CHAIN (pp_val) != NULL_TREE)
> > > > > > +     {
> > > > > > +       tree patchable_function_entry_value2
> > > > > > +         = TREE_VALUE (TREE_CHAIN (pp_val));
> > > > > > +       patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> > > > > > +     }
> > > > > > +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> > > > > > +     error ("invalid values for %<patchable_function_entry%> attribute");
> > > > >
> > > > > This should probably go in handle_patchable_function_entry_attribute
> > > > > instead.  It doesn't look like we have a clear policy about whether
> > > > > errors or warnings are right for unrecognised arguments to known
> > > > > attribute names, but handle_patchable_function_entry_attribute reports
> > > > > an OPT_Wattributes warning for arguments that aren't integers.  Doing the
> > > > > same for out-of-range integers would be more consistent and means that
> > > > > we wouldn't break existing code if we relaxed/changed the rules in future.
> > > >
> > > > Like this?  OK for master if there is no regression?
> > > >
> > >
> > > There is a small problem.  Warnings from C and C++ frond-ends are different:
> > >
> > > [hjl@gnu-skx-1 gcc]$ cat x.c
> > > void
> > >  __attribute__((patchable_function_entry(65536)))
> > > foo1 (void)
> > > { /* { dg-warning "'patchable_function_entry' attribute argument
> > > '65536' is out of range" } */
> > > }
> > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
> > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
> > > ‘65536’ is out of range (> 65535) [-Wattributes]
> > >     4 | { /* { dg-warning "'patchable_function_entry' attribute
> > > argument '65536' is out of range" } */
> > >       | ^
> > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
> > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
> > > ‘65536’ is out of range (> 65535) [-Wattributes]
> > >     3 | foo1 (void)
> > >       |           ^
> > > [hjl@gnu-skx-1 gcc]$
> > >
> > > C warns at line 4 and C++ warns at line 3.   Do I need separate tests
> > > for C and C++?
> >
> > I think better would be
> >
> > /* { dg-error "foo" "" { target c } } */
> > /* { dg-error "bar" "" { target c++ } .-1 } */
> >
> > Marek
> >
>
> It worked.

Here is the patch with updated tests.   There are no regressions on
Linux/x86-64.
OK for master?

Thanks.


-- 
H.J.

[-- Attachment #2: 0001-Add-patch_area_size-and-patch_area_entry-to-crtl.patch --]
[-- Type: text/x-patch, Size: 10156 bytes --]

From 8320bbb83e53f9e135fe1eca50840baacf157881 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Feb 2020 04:01:59 -0800
Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl

Currently patchable area is at the wrong place.  It is placed immediately
after function label and before .cfi_startproc.  A backend should be able
to add a pseudo patchable area instruction durectly into RTL.  This patch
adds patch_area_size and patch_area_entry to crtl so that the patchable
area info is available in RTL passes.

It also limits patch_area_size and patch_area_entry to 65535, which is
a reasonable maximum size for patchable area.

gcc/c-family/

	PR target/93492
	* c-attribs.c (handle_patchable_function_entry_attribute): Limit
	value to USHRT_MAX (65535).

gcc/

	PR target/93492
	* cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size
	and crtl->patch_area_entry.
	* emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry.
	* opts.c (common_handle_option): Limit
	function_entry_patch_area_size and function_entry_patch_area_start
	to USHRT_MAX.  Fix a typo in error message.
	* varasm.c (assemble_start_function): Use crtl->patch_area_size
	and crtl->patch_area_entry.
	* doc/invoke.texi: Document the maximum value for
	-fpatchable-function-entry.

gcc/testsuite/

	PR target/93492
	* c-c++-common/patchable_function_entry-error-1.c: New test.
	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
	* c-c++-common/patchable_function_entry-error-3.c: Likewise.
---
 gcc/c-family/c-attribs.c                      |  9 +++++
 gcc/cfgexpand.c                               | 33 +++++++++++++++++++
 gcc/doc/invoke.texi                           |  1 +
 gcc/emit-rtl.h                                |  6 ++++
 gcc/opts.c                                    |  4 ++-
 .../patchable_function_entry-error-1.c        |  9 +++++
 .../patchable_function_entry-error-2.c        |  9 +++++
 .../patchable_function_entry-error-3.c        | 20 +++++++++++
 gcc/varasm.c                                  | 30 ++---------------
 9 files changed, 92 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 7ec6fc848ac..15dbda1eff7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4539,6 +4539,15 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
 	  *no_add_attrs = true;
 	  return NULL_TREE;
 	}
+
+      if (tree_to_uhwi (val) > USHRT_MAX)
+	{
+	  warning (OPT_Wattributes,
+		   "%qE attribute argument %qE is out of range (> 65535)",
+		   name, val);
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
     }
   return NULL_TREE;
 }
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 9864e4344d2..f063f50c263 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -6656,6 +6656,39 @@ pass_expand::execute (function *fun)
   if (crtl->tail_call_emit)
     fixup_tail_calls ();
 
+  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+  tree patchable_function_entry_attr
+    = lookup_attribute ("patchable_function_entry",
+			DECL_ATTRIBUTES (cfun->decl));
+  if (patchable_function_entry_attr)
+    {
+      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+      patch_area_entry = 0;
+      if (TREE_CHAIN (pp_val) != NULL_TREE)
+	{
+	  tree patchable_function_entry_value2
+	    = TREE_VALUE (TREE_CHAIN (pp_val));
+	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+	}
+    }
+
+  if (patch_area_entry > patch_area_size)
+    {
+      if (patch_area_size > 0)
+	warning (OPT_Wattributes,
+		 "patchable function entry %wu exceeds size %wu",
+		 patch_area_entry, patch_area_size);
+      patch_area_entry = 0;
+    }
+
+  crtl->patch_area_size = patch_area_size;
+  crtl->patch_area_entry = patch_area_entry;
+
   /* BB subdivision may have created basic blocks that are are only reachable
      from unlikely bbs but not marked as such in the profile.  */
   if (optimize)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 35b341e759f..dd4835199b0 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
 The NOP instructions are inserted at---and maybe before, depending on
 @var{M}---the function entry address, even before the prologue.
 
+The maximum value of @var{N} and @var{M} is 65535.
 @end table
 
 
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index a878efe3cf7..3d6565c8a30 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -173,6 +173,12 @@ struct GTY(()) rtl_data {
         local stack.  */
   unsigned int stack_alignment_estimated;
 
+  /* How many NOP insns to place at each function entry by default.  */
+  unsigned short patch_area_size;
+
+  /* How far the real asm entry point is into this area.  */
+  unsigned short patch_area_entry;
+
   /* For reorg.  */
 
   /* Nonzero if function being compiled called builtin_return_addr or
diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb41a96..c6011f1f9b7 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2598,10 +2598,12 @@ common_handle_option (struct gcc_options *opts,
 	    function_entry_patch_area_start = 0;
 	  }
 	if (function_entry_patch_area_size < 0
+	    || function_entry_patch_area_size > USHRT_MAX
 	    || function_entry_patch_area_start < 0
+	    || function_entry_patch_area_start > USHRT_MAX
 	    || function_entry_patch_area_size 
 		< function_entry_patch_area_start)
-	  error ("invalid arguments for %<-fpatchable_function_entry%>");
+	  error ("invalid arguments for %<-fpatchable-function-entry%>");
 	free (patch_area_arg);
       }
       break;
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
new file mode 100644
index 00000000000..f60bf46cfe3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=65536,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
new file mode 100644
index 00000000000..90f88c78be7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=1,65536" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
new file mode 100644
index 00000000000..b3daeabcbb6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+
+void
+ __attribute__((patchable_function_entry(65536)))
+foo1 (void) /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c++ } } */
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c } } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,1)))
+foo2 (void) /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c++ } } */
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c } } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,65536)))
+foo3 (void) /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c++ } } */
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" "" { target c } } */
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index dc6da6c0b5b..9179fecdf85 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1857,34 +1857,8 @@ 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);
-
-      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
-      patch_area_entry = 0;
-      if (TREE_CHAIN (pp_val) != NULL_TREE)
-	{
-	  tree patchable_function_entry_value2
-	    = TREE_VALUE (TREE_CHAIN (pp_val));
-	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
-	}
-    }
-
-  if (patch_area_entry > patch_area_size)
-    {
-      if (patch_area_size > 0)
-	warning (OPT_Wattributes,
-		 "patchable function entry %wu exceeds size %wu",
-		 patch_area_entry, patch_area_size);
-      patch_area_entry = 0;
-    }
+  unsigned short patch_area_size = crtl->patch_area_size;
+  unsigned short patch_area_entry = crtl->patch_area_entry;
 
   /* Emit the patching area before the entry label, if any.  */
   if (patch_area_entry > 0)
-- 
2.24.1

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[Richard Sandiford]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote:
>> >
>> > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote:
>> > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>> > > >
>> > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford
>> > > > <richard.sandiford@arm.com> wrote:
>> > > > >
>> > > > > "H.J. Lu" <hjl.tools@gmail.com> writes:
>> > > > > > Currently patchable area is at the wrong place.
>> > > > >
>> > > > > Agreed :-)
>> > > > >
>> > > > > > It is placed immediately
>> > > > > > after function label and before .cfi_startproc.  A backend should be able
>> > > > > > to add a pseudo patchable area instruction durectly into RTL.  This patch
>> > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable
>> > > > > > area info is available in RTL passes.
>> > > > >
>> > > > > It might be better to add it to crtl, since it should only be needed
>> > > > > during rtl generation.
>> > > > >
>> > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is
>> > > > > > a reasonable maximum size for patchable area.
>> > > > > >
>> > > > > > gcc/
>> > > > > >
>> > > > > >       PR target/93492
>> > > > > >       * function.c (expand_function_start): Set cfun->patch_area_size
>> > > > > >       and cfun->patch_area_entry.
>> > > > > >       * function.h (function): Add patch_area_size and patch_area_entry.
>> > > > > >       * opts.c (common_handle_option): Limit
>> > > > > >       function_entry_patch_area_size and function_entry_patch_area_start
>> > > > > >       to USHRT_MAX.  Fix a typo in error message.
>> > > > > >       * varasm.c (assemble_start_function): Use cfun->patch_area_size
>> > > > > >       and cfun->patch_area_entry.
>> > > > > >       * doc/invoke.texi: Document the maximum value for
>> > > > > >       -fpatchable-function-entry.
>> > > > > >
>> > > > > > gcc/testsuite/
>> > > > > >
>> > > > > >       PR target/93492
>> > > > > >       * c-c++-common/patchable_function_entry-error-1.c: New test.
>> > > > > >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
>> > > > > >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
>> > > > > > ---
>> > > > > >  gcc/doc/invoke.texi                           |  1 +
>> > > > > >  gcc/function.c                                | 35 +++++++++++++++++++
>> > > > > >  gcc/function.h                                |  6 ++++
>> > > > > >  gcc/opts.c                                    |  4 ++-
>> > > > > >  .../patchable_function_entry-error-1.c        |  9 +++++
>> > > > > >  .../patchable_function_entry-error-2.c        |  9 +++++
>> > > > > >  .../patchable_function_entry-error-3.c        | 20 +++++++++++
>> > > > > >  gcc/varasm.c                                  | 30 ++--------------
>> > > > > >  8 files changed, 85 insertions(+), 29 deletions(-)
>> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
>> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
>> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
>> > > > > >
>> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> > > > > > index 35b341e759f..dd4835199b0 100644
>> > > > > > --- a/gcc/doc/invoke.texi
>> > > > > > +++ b/gcc/doc/invoke.texi
>> > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
>> > > > > >  The NOP instructions are inserted at---and maybe before, depending on
>> > > > > >  @var{M}---the function entry address, even before the prologue.
>> > > > > >
>> > > > > > +The maximum value of @var{N} and @var{M} is 65535.
>> > > > > >  @end table
>> > > > > >
>> > > > > >
>> > > > > > diff --git a/gcc/function.c b/gcc/function.c
>> > > > > > index d8008f60422..badbf538eec 100644
>> > > > > > --- a/gcc/function.c
>> > > > > > +++ b/gcc/function.c
>> > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
>> > > > > >    /* If we are doing generic stack checking, the probe should go here.  */
>> > > > > >    if (flag_stack_check == GENERIC_STACK_CHECK)
>> > > > > >      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
>> > > > > > +
>> > > > > > +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
>> > > > > > +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
>> > > > > > +
>> > > > > > +  tree patchable_function_entry_attr
>> > > > > > +    = lookup_attribute ("patchable_function_entry",
>> > > > > > +                     DECL_ATTRIBUTES (cfun->decl));
>> > > > > > +  if (patchable_function_entry_attr)
>> > > > > > +    {
>> > > > > > +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
>> > > > > > +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
>> > > > > > +
>> > > > > > +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
>> > > > > > +      patch_area_entry = 0;
>> > > > > > +      if (TREE_CHAIN (pp_val) != NULL_TREE)
>> > > > > > +     {
>> > > > > > +       tree patchable_function_entry_value2
>> > > > > > +         = TREE_VALUE (TREE_CHAIN (pp_val));
>> > > > > > +       patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
>> > > > > > +     }
>> > > > > > +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
>> > > > > > +     error ("invalid values for %<patchable_function_entry%> attribute");
>> > > > >
>> > > > > This should probably go in handle_patchable_function_entry_attribute
>> > > > > instead.  It doesn't look like we have a clear policy about whether
>> > > > > errors or warnings are right for unrecognised arguments to known
>> > > > > attribute names, but handle_patchable_function_entry_attribute reports
>> > > > > an OPT_Wattributes warning for arguments that aren't integers.  Doing the
>> > > > > same for out-of-range integers would be more consistent and means that
>> > > > > we wouldn't break existing code if we relaxed/changed the rules in future.
>> > > >
>> > > > Like this?  OK for master if there is no regression?
>> > > >
>> > >
>> > > There is a small problem.  Warnings from C and C++ frond-ends are different:
>> > >
>> > > [hjl@gnu-skx-1 gcc]$ cat x.c
>> > > void
>> > >  __attribute__((patchable_function_entry(65536)))
>> > > foo1 (void)
>> > > { /* { dg-warning "'patchable_function_entry' attribute argument
>> > > '65536' is out of range" } */
>> > > }
>> > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
>> > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
>> > > ‘65536’ is out of range (> 65535) [-Wattributes]
>> > >     4 | { /* { dg-warning "'patchable_function_entry' attribute
>> > > argument '65536' is out of range" } */
>> > >       | ^
>> > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
>> > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
>> > > ‘65536’ is out of range (> 65535) [-Wattributes]
>> > >     3 | foo1 (void)
>> > >       |           ^
>> > > [hjl@gnu-skx-1 gcc]$
>> > >
>> > > C warns at line 4 and C++ warns at line 3.   Do I need separate tests
>> > > for C and C++?
>> >
>> > I think better would be
>> >
>> > /* { dg-error "foo" "" { target c } } */
>> > /* { dg-error "bar" "" { target c++ } .-1 } */
>> >
>> > Marek
>> >
>>
>> It worked.
>
> Here is the patch with updated tests.   There are no regressions on
> Linux/x86-64.
> OK for master?
>
> Thanks.
>
>
> -- 
> H.J.
>
> From 8320bbb83e53f9e135fe1eca50840baacf157881 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Wed, 5 Feb 2020 04:01:59 -0800
> Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl
>
> Currently patchable area is at the wrong place.  It is placed immediately
> after function label and before .cfi_startproc.  A backend should be able
> to add a pseudo patchable area instruction durectly into RTL.  This patch
> adds patch_area_size and patch_area_entry to crtl so that the patchable
> area info is available in RTL passes.
>
> It also limits patch_area_size and patch_area_entry to 65535, which is
> a reasonable maximum size for patchable area.
>
> gcc/c-family/
>
> 	PR target/93492
> 	* c-attribs.c (handle_patchable_function_entry_attribute): Limit
> 	value to USHRT_MAX (65535).
>
> gcc/
>
> 	PR target/93492
> 	* cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size
> 	and crtl->patch_area_entry.
> 	* emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry.
> 	* opts.c (common_handle_option): Limit
> 	function_entry_patch_area_size and function_entry_patch_area_start
> 	to USHRT_MAX.  Fix a typo in error message.
> 	* varasm.c (assemble_start_function): Use crtl->patch_area_size
> 	and crtl->patch_area_entry.
> 	* doc/invoke.texi: Document the maximum value for
> 	-fpatchable-function-entry.
>
> gcc/testsuite/
>
> 	PR target/93492
> 	* c-c++-common/patchable_function_entry-error-1.c: New test.
> 	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
> 	* c-c++-common/patchable_function_entry-error-3.c: Likewise.

LGTM, but an RM should have the final say on whether this is
suitable for stage 4.

Thanks,
Richard

Re: [PATCH] Add patch_area_size and patch_area_entry to crtl

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 10103 bytes --]

On Thu, Feb 6, 2020 at 12:01 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
> > On Wed, Feb 5, 2020 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Wed, Feb 5, 2020 at 2:37 PM Marek Polacek <polacek@redhat.com> wrote:
> >> >
> >> > On Wed, Feb 05, 2020 at 02:24:48PM -0800, H.J. Lu wrote:
> >> > > On Wed, Feb 5, 2020 at 12:20 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> > > >
> >> > > > On Wed, Feb 5, 2020 at 9:00 AM Richard Sandiford
> >> > > > <richard.sandiford@arm.com> wrote:
> >> > > > >
> >> > > > > "H.J. Lu" <hjl.tools@gmail.com> writes:
> >> > > > > > Currently patchable area is at the wrong place.
> >> > > > >
> >> > > > > Agreed :-)
> >> > > > >
> >> > > > > > It is placed immediately
> >> > > > > > after function label and before .cfi_startproc.  A backend should be able
> >> > > > > > to add a pseudo patchable area instruction durectly into RTL.  This patch
> >> > > > > > adds patch_area_size and patch_area_entry to cfun so that the patchable
> >> > > > > > area info is available in RTL passes.
> >> > > > >
> >> > > > > It might be better to add it to crtl, since it should only be needed
> >> > > > > during rtl generation.
> >> > > > >
> >> > > > > > It also limits patch_area_size and patch_area_entry to 65535, which is
> >> > > > > > a reasonable maximum size for patchable area.
> >> > > > > >
> >> > > > > > gcc/
> >> > > > > >
> >> > > > > >       PR target/93492
> >> > > > > >       * function.c (expand_function_start): Set cfun->patch_area_size
> >> > > > > >       and cfun->patch_area_entry.
> >> > > > > >       * function.h (function): Add patch_area_size and patch_area_entry.
> >> > > > > >       * opts.c (common_handle_option): Limit
> >> > > > > >       function_entry_patch_area_size and function_entry_patch_area_start
> >> > > > > >       to USHRT_MAX.  Fix a typo in error message.
> >> > > > > >       * varasm.c (assemble_start_function): Use cfun->patch_area_size
> >> > > > > >       and cfun->patch_area_entry.
> >> > > > > >       * doc/invoke.texi: Document the maximum value for
> >> > > > > >       -fpatchable-function-entry.
> >> > > > > >
> >> > > > > > gcc/testsuite/
> >> > > > > >
> >> > > > > >       PR target/93492
> >> > > > > >       * c-c++-common/patchable_function_entry-error-1.c: New test.
> >> > > > > >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> >> > > > > >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
> >> > > > > > ---
> >> > > > > >  gcc/doc/invoke.texi                           |  1 +
> >> > > > > >  gcc/function.c                                | 35 +++++++++++++++++++
> >> > > > > >  gcc/function.h                                |  6 ++++
> >> > > > > >  gcc/opts.c                                    |  4 ++-
> >> > > > > >  .../patchable_function_entry-error-1.c        |  9 +++++
> >> > > > > >  .../patchable_function_entry-error-2.c        |  9 +++++
> >> > > > > >  .../patchable_function_entry-error-3.c        | 20 +++++++++++
> >> > > > > >  gcc/varasm.c                                  | 30 ++--------------
> >> > > > > >  8 files changed, 85 insertions(+), 29 deletions(-)
> >> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
> >> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
> >> > > > > >  create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
> >> > > > > >
> >> > > > > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> >> > > > > > index 35b341e759f..dd4835199b0 100644
> >> > > > > > --- a/gcc/doc/invoke.texi
> >> > > > > > +++ b/gcc/doc/invoke.texi
> >> > > > > > @@ -13966,6 +13966,7 @@ If @code{N=0}, no pad location is recorded.
> >> > > > > >  The NOP instructions are inserted at---and maybe before, depending on
> >> > > > > >  @var{M}---the function entry address, even before the prologue.
> >> > > > > >
> >> > > > > > +The maximum value of @var{N} and @var{M} is 65535.
> >> > > > > >  @end table
> >> > > > > >
> >> > > > > >
> >> > > > > > diff --git a/gcc/function.c b/gcc/function.c
> >> > > > > > index d8008f60422..badbf538eec 100644
> >> > > > > > --- a/gcc/function.c
> >> > > > > > +++ b/gcc/function.c
> >> > > > > > @@ -5202,6 +5202,41 @@ expand_function_start (tree subr)
> >> > > > > >    /* If we are doing generic stack checking, the probe should go here.  */
> >> > > > > >    if (flag_stack_check == GENERIC_STACK_CHECK)
> >> > > > > >      stack_check_probe_note = emit_note (NOTE_INSN_DELETED);
> >> > > > > > +
> >> > > > > > +  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
> >> > > > > > +  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
> >> > > > > > +
> >> > > > > > +  tree patchable_function_entry_attr
> >> > > > > > +    = lookup_attribute ("patchable_function_entry",
> >> > > > > > +                     DECL_ATTRIBUTES (cfun->decl));
> >> > > > > > +  if (patchable_function_entry_attr)
> >> > > > > > +    {
> >> > > > > > +      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
> >> > > > > > +      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
> >> > > > > > +
> >> > > > > > +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> >> > > > > > +      patch_area_entry = 0;
> >> > > > > > +      if (TREE_CHAIN (pp_val) != NULL_TREE)
> >> > > > > > +     {
> >> > > > > > +       tree patchable_function_entry_value2
> >> > > > > > +         = TREE_VALUE (TREE_CHAIN (pp_val));
> >> > > > > > +       patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> >> > > > > > +     }
> >> > > > > > +      if (patch_area_size > USHRT_MAX || patch_area_size > USHRT_MAX)
> >> > > > > > +     error ("invalid values for %<patchable_function_entry%> attribute");
> >> > > > >
> >> > > > > This should probably go in handle_patchable_function_entry_attribute
> >> > > > > instead.  It doesn't look like we have a clear policy about whether
> >> > > > > errors or warnings are right for unrecognised arguments to known
> >> > > > > attribute names, but handle_patchable_function_entry_attribute reports
> >> > > > > an OPT_Wattributes warning for arguments that aren't integers.  Doing the
> >> > > > > same for out-of-range integers would be more consistent and means that
> >> > > > > we wouldn't break existing code if we relaxed/changed the rules in future.
> >> > > >
> >> > > > Like this?  OK for master if there is no regression?
> >> > > >
> >> > >
> >> > > There is a small problem.  Warnings from C and C++ frond-ends are different:
> >> > >
> >> > > [hjl@gnu-skx-1 gcc]$ cat x.c
> >> > > void
> >> > >  __attribute__((patchable_function_entry(65536)))
> >> > > foo1 (void)
> >> > > { /* { dg-warning "'patchable_function_entry' attribute argument
> >> > > '65536' is out of range" } */
> >> > > }
> >> > > [hjl@gnu-skx-1 gcc]$ ./xgcc -B./ -S x.c
> >> > > x.c:4:1: warning: ‘patchable_function_entry’ attribute argument
> >> > > ‘65536’ is out of range (> 65535) [-Wattributes]
> >> > >     4 | { /* { dg-warning "'patchable_function_entry' attribute
> >> > > argument '65536' is out of range" } */
> >> > >       | ^
> >> > > [hjl@gnu-skx-1 gcc]$ ./xg++ -B./ -S x.c
> >> > > x.c:3:11: warning: ‘patchable_function_entry’ attribute argument
> >> > > ‘65536’ is out of range (> 65535) [-Wattributes]
> >> > >     3 | foo1 (void)
> >> > >       |           ^
> >> > > [hjl@gnu-skx-1 gcc]$
> >> > >
> >> > > C warns at line 4 and C++ warns at line 3.   Do I need separate tests
> >> > > for C and C++?
> >> >
> >> > I think better would be
> >> >
> >> > /* { dg-error "foo" "" { target c } } */
> >> > /* { dg-error "bar" "" { target c++ } .-1 } */
> >> >
> >> > Marek
> >> >
> >>
> >> It worked.
> >
> > Here is the patch with updated tests.   There are no regressions on
> > Linux/x86-64.
> > OK for master?
> >
> > Thanks.
> >
> >
> > --
> > H.J.
> >
> > From 8320bbb83e53f9e135fe1eca50840baacf157881 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Wed, 5 Feb 2020 04:01:59 -0800
> > Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl
> >
> > Currently patchable area is at the wrong place.  It is placed immediately
> > after function label and before .cfi_startproc.  A backend should be able
> > to add a pseudo patchable area instruction durectly into RTL.  This patch
> > adds patch_area_size and patch_area_entry to crtl so that the patchable
> > area info is available in RTL passes.
> >
> > It also limits patch_area_size and patch_area_entry to 65535, which is
> > a reasonable maximum size for patchable area.
> >
> > gcc/c-family/
> >
> >       PR target/93492
> >       * c-attribs.c (handle_patchable_function_entry_attribute): Limit
> >       value to USHRT_MAX (65535).
> >
> > gcc/
> >
> >       PR target/93492
> >       * cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size
> >       and crtl->patch_area_entry.
> >       * emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry.
> >       * opts.c (common_handle_option): Limit
> >       function_entry_patch_area_size and function_entry_patch_area_start
> >       to USHRT_MAX.  Fix a typo in error message.
> >       * varasm.c (assemble_start_function): Use crtl->patch_area_size
> >       and crtl->patch_area_entry.
> >       * doc/invoke.texi: Document the maximum value for
> >       -fpatchable-function-entry.
> >
> > gcc/testsuite/
> >
> >       PR target/93492
> >       * c-c++-common/patchable_function_entry-error-1.c: New test.
> >       * c-c++-common/patchable_function_entry-error-2.c: Likewise.
> >       * c-c++-common/patchable_function_entry-error-3.c: Likewise.
>
> LGTM, but an RM should have the final say on whether this is
> suitable for stage 4.
>

This is the patch I am checking in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-Add-patch_area_size-and-patch_area_entry-to-crtl.patch --]
[-- Type: text/x-patch, Size: 9771 bytes --]

From f4a89f5c9beb31f707c970db0320e4687e962c88 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 5 Feb 2020 04:01:59 -0800
Subject: [PATCH] Add patch_area_size and patch_area_entry to crtl

Currently patchable area is at the wrong place.  It is placed immediately
after function label and before .cfi_startproc.  A backend should be able
to add a pseudo patchable area instruction durectly into RTL.  This patch
adds patch_area_size and patch_area_entry to crtl so that the patchable
area info is available in RTL passes.

It also limits patch_area_size and patch_area_entry to 65535, which is
a reasonable maximum size for patchable area.

gcc/c-family/

	PR target/93492
	* c-attribs.c (handle_patchable_function_entry_attribute): Limit
	value to USHRT_MAX (65535).

gcc/

	PR target/93492
	* cfgexpand.c (pass_expand::execute): Set crtl->patch_area_size
	and crtl->patch_area_entry.
	* emit-rtl.h (rtl_data): Add patch_area_size and patch_area_entry.
	* opts.c (common_handle_option): Limit
	function_entry_patch_area_size and function_entry_patch_area_start
	to USHRT_MAX.  Fix a typo in error message.
	* varasm.c (assemble_start_function): Use crtl->patch_area_size
	and crtl->patch_area_entry.
	* doc/invoke.texi: Document the maximum value for
	-fpatchable-function-entry.

gcc/testsuite/

	PR target/93492
	* c-c++-common/patchable_function_entry-error-1.c: New test.
	* c-c++-common/patchable_function_entry-error-2.c: Likewise.
	* c-c++-common/patchable_function_entry-error-3.c: Likewise.
---
 gcc/c-family/c-attribs.c                      |  9 +++++
 gcc/cfgexpand.c                               | 33 +++++++++++++++++++
 gcc/doc/invoke.texi                           |  1 +
 gcc/emit-rtl.h                                |  6 ++++
 gcc/opts.c                                    |  4 ++-
 .../patchable_function_entry-error-1.c        |  9 +++++
 .../patchable_function_entry-error-2.c        |  9 +++++
 .../patchable_function_entry-error-3.c        | 20 +++++++++++
 gcc/varasm.c                                  | 30 ++---------------
 9 files changed, 92 insertions(+), 29 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
 create mode 100644 gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index ac936d5bbbb..a101312c581 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -4553,6 +4553,15 @@ handle_patchable_function_entry_attribute (tree *, tree name, tree args,
 	  *no_add_attrs = true;
 	  return NULL_TREE;
 	}
+
+      if (tree_to_uhwi (val) > USHRT_MAX)
+	{
+	  warning (OPT_Wattributes,
+		   "%qE attribute argument %qE is out of range (> 65535)",
+		   name, val);
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
     }
   return NULL_TREE;
 }
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index a7ec77d5c85..86efa22bf60 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -6656,6 +6656,39 @@ pass_expand::execute (function *fun)
   if (crtl->tail_call_emit)
     fixup_tail_calls ();
 
+  unsigned HOST_WIDE_INT patch_area_size = function_entry_patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = function_entry_patch_area_start;
+
+  tree patchable_function_entry_attr
+    = lookup_attribute ("patchable_function_entry",
+			DECL_ATTRIBUTES (cfun->decl));
+  if (patchable_function_entry_attr)
+    {
+      tree pp_val = TREE_VALUE (patchable_function_entry_attr);
+      tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
+
+      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
+      patch_area_entry = 0;
+      if (TREE_CHAIN (pp_val) != NULL_TREE)
+	{
+	  tree patchable_function_entry_value2
+	    = TREE_VALUE (TREE_CHAIN (pp_val));
+	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
+	}
+    }
+
+  if (patch_area_entry > patch_area_size)
+    {
+      if (patch_area_size > 0)
+	warning (OPT_Wattributes,
+		 "patchable function entry %wu exceeds size %wu",
+		 patch_area_entry, patch_area_size);
+      patch_area_entry = 0;
+    }
+
+  crtl->patch_area_size = patch_area_size;
+  crtl->patch_area_entry = patch_area_entry;
+
   /* BB subdivision may have created basic blocks that are only reachable
      from unlikely bbs but not marked as such in the profile.  */
   if (optimize)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 527d362533a..767d1f07801 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -14112,6 +14112,7 @@ If @code{N=0}, no pad location is recorded.
 The NOP instructions are inserted at---and maybe before, depending on
 @var{M}---the function entry address, even before the prologue.
 
+The maximum value of @var{N} and @var{M} is 65535.
 @end table
 
 
diff --git a/gcc/emit-rtl.h b/gcc/emit-rtl.h
index a878efe3cf7..3d6565c8a30 100644
--- a/gcc/emit-rtl.h
+++ b/gcc/emit-rtl.h
@@ -173,6 +173,12 @@ struct GTY(()) rtl_data {
         local stack.  */
   unsigned int stack_alignment_estimated;
 
+  /* How many NOP insns to place at each function entry by default.  */
+  unsigned short patch_area_size;
+
+  /* How far the real asm entry point is into this area.  */
+  unsigned short patch_area_entry;
+
   /* For reorg.  */
 
   /* Nonzero if function being compiled called builtin_return_addr or
diff --git a/gcc/opts.c b/gcc/opts.c
index c212a1a57dc..3dccef39701 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2615,10 +2615,12 @@ common_handle_option (struct gcc_options *opts,
 	    function_entry_patch_area_start = 0;
 	  }
 	if (function_entry_patch_area_size < 0
+	    || function_entry_patch_area_size > USHRT_MAX
 	    || function_entry_patch_area_start < 0
+	    || function_entry_patch_area_start > USHRT_MAX
 	    || function_entry_patch_area_size 
 		< function_entry_patch_area_start)
-	  error ("invalid arguments for %<-fpatchable_function_entry%>");
+	  error ("invalid arguments for %<-fpatchable-function-entry%>");
 	free (patch_area_arg);
       }
       break;
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
new file mode 100644
index 00000000000..f60bf46cfe3
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=65536,1" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
new file mode 100644
index 00000000000..90f88c78be7
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-options "-O2 -fpatchable-function-entry=1,65536" } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+/* { dg-error "invalid arguments for '-fpatchable-function-entry'" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
new file mode 100644
index 00000000000..45e93988886
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/patchable_function_entry-error-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile { target { ! { nvptx*-*-* visium-*-* } } } } */
+/* { dg-additional-options "-fno-pie" { target sparc*-*-* } } */
+
+void
+ __attribute__((patchable_function_entry(65536)))
+foo1 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,1)))
+foo2 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
+
+void
+ __attribute__((patchable_function_entry(65536,65536)))
+foo3 (void)
+{ /* { dg-warning "'patchable_function_entry' attribute argument '65536' is out of range" } */
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 271a67abf56..f062e48071f 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1857,34 +1857,8 @@ 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);
-
-      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
-      patch_area_entry = 0;
-      if (TREE_CHAIN (pp_val) != NULL_TREE)
-	{
-	  tree patchable_function_entry_value2
-	    = TREE_VALUE (TREE_CHAIN (pp_val));
-	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
-	}
-    }
-
-  if (patch_area_entry > patch_area_size)
-    {
-      if (patch_area_size > 0)
-	warning (OPT_Wattributes,
-		 "patchable function entry %wu exceeds size %wu",
-		 patch_area_entry, patch_area_size);
-      patch_area_entry = 0;
-    }
+  unsigned short patch_area_size = crtl->patch_area_size;
+  unsigned short patch_area_entry = crtl->patch_area_entry;
 
   /* Emit the patching area before the entry label, if any.  */
   if (patch_area_entry > 0)
-- 
2.26.2