Re: [PATCH] MIPS: Fixed the problem that the nop instruction is inserted at the wrong position after enabling '-fpatchable-function-entry='

[WANG Xuerui]

On 4/30/25 14:26, Lulu Cheng wrote:

Because MIPS function symbol is generated in the prologue function,
this nop generation should be done in prologue.

OK for trunk?

        PR target/99217
gcc/ChangeLog:

        * config/mips/mips.cc (mips_start_function_definition):
        Implements the functionality of '-fpatchable-function-entry='.
        (mips_print_patchable_function_entry): Define empty function.
        (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): Define macro.

Change-Id: If156e635568edb6560d9230f6967e01d207d46b2

Remove this presumably internal tag?

---
  gcc/config/mips/mips.cc | 32 ++++++++++++++++++++++++++++++++
  1 file changed, 32 insertions(+)

diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 492fa285477..7478e376fdf 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -7478,6 +7478,9 @@ static void
  mips_start_function_definition (const char *name, bool mips16_p,
                                tree decl ATTRIBUTE_UNUSED)
  {
+  unsigned HOST_WIDE_INT patch_area_size = crtl->patch_area_size;
+  unsigned HOST_WIDE_INT patch_area_entry = crtl->patch_area_entry;
+
    if (mips16_p)
      fprintf (asm_out_file, "\t.set\tmips16\n");
    else
@@ -7490,6 +7493,10 @@ mips_start_function_definition (const char *name, bool 
mips16_p,
      fprintf (asm_out_file, "\t.set\tnomicromips\n");
  #endif

+ /* Emit the patching area before the entry label, if any. */

+  if (patch_area_entry > 0)
+    default_print_patchable_function_entry (asm_out_file,
+                                           patch_area_entry, true);
    if (!flag_inhibit_size_directive)
      {
        fputs ("\t.ent\t", asm_out_file);
@@ -7501,6 +7508,13 @@ mips_start_function_definition (const char *name, bool 
mips16_p,

/* Start the definition proper. */

    ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, name, decl);
+
+  /* And the area after the label.  Record it if we haven't done so yet.  */
+  if (patch_area_size > patch_area_entry)
+    default_print_patchable_function_entry (asm_out_file,
+                                           patch_area_size
+                                           - patch_area_entry,
+                                           patch_area_entry == 0);
  }

/* End a function definition started by mips_start_function_definition. */

@@ -23314,6 +23328,20 @@ mips_bit_clear_p (enum machine_mode mode, unsigned 
HOST_WIDE_INT m)
    return false;
  }

+/* define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY */

+
+/* The MIPS function start is implemented in the prologue function.
+   TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY needs to be inserted
+   before or after the function name, so this function does not
+   use a public implementation. This function is implemented in
+   mips_start_function_definition. */
+
+void
+mips_print_patchable_function_entry (FILE *file,
+                                    unsigned HOST_WIDE_INT patch_area_size,
+                                    bool record_p)
+{}
+
  /* Initialize the GCC target structure.  */
  #undef TARGET_ASM_ALIGNED_HI_OP
  #define TARGET_ASM_ALIGNED_HI_OP "\t.half\t"
@@ -23627,6 +23655,10 @@ mips_bit_clear_p (enum machine_mode mode, unsigned 
HOST_WIDE_INT m)
  #undef TARGET_DOCUMENTATION_NAME
  #define TARGET_DOCUMENTATION_NAME "MIPS"

+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+mips_print_patchable_function_entry
+
  struct gcc_target targetm = TARGET_INITIALIZER;
  
  #include "gt-mips.h"

A test case would greatly help, otherwise LGTM. Thanks!