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

[Lulu Cheng]

在 2025/5/6 下午6:14, 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?

Sorry, I forgot to delete this.

---
  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!

Ok, I will add test cases in the next version.

Thanks!