[v2,1/2] Implement ASM_DECLARE_FUNCTION_NAME using ASM_OUTPUT_FUNCTION_LABEL

Message ID 20240102194511.3171559-2-iii@linux.ibm.com
State Unresolved
Headers
Series asan: Align .LASANPC on function boundary |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Ilya Leoshkevich Jan. 2, 2024, 7:41 p.m. UTC
  gccint recommends using ASM_OUTPUT_FUNCTION_LABEL in
ASM_DECLARE_FUNCTION_NAME, but many implementations use
ASM_OUTPUT_LABEL instead.  It's inconsistent and prevents changes to
ASM_OUTPUT_FUNCTION_LABEL from affecting the respective targets.
---
 gcc/config/aarch64/aarch64.cc       |  2 +-
 gcc/config/alpha/alpha.cc           |  5 ++---
 gcc/config/arm/aout.h               |  2 +-
 gcc/config/arm/arm.cc               |  2 +-
 gcc/config/bfin/bfin.h              | 16 ++++++++--------
 gcc/config/c6x/c6x.h                |  2 +-
 gcc/config/gcn/gcn.cc               |  5 ++---
 gcc/config/h8300/h8300.h            |  2 +-
 gcc/config/ia64/ia64.cc             |  5 ++---
 gcc/config/mcore/mcore-elf.h        |  2 +-
 gcc/config/microblaze/microblaze.cc |  3 +--
 gcc/config/mips/mips.cc             | 19 ++++++++++---------
 gcc/config/pa/pa.cc                 |  3 ++-
 gcc/config/riscv/riscv.cc           |  2 +-
 gcc/config/rs6000/rs6000.cc         |  4 ++--
 15 files changed, 36 insertions(+), 38 deletions(-)
  

Comments

Jeff Law Jan. 4, 2024, 7:47 p.m. UTC | #1
On 1/2/24 12:41, Ilya Leoshkevich wrote:
> gccint recommends using ASM_OUTPUT_FUNCTION_LABEL in
> ASM_DECLARE_FUNCTION_NAME, but many implementations use
> ASM_OUTPUT_LABEL instead.  It's inconsistent and prevents changes to
> ASM_OUTPUT_FUNCTION_LABEL from affecting the respective targets.
> ---
>   gcc/config/aarch64/aarch64.cc       |  2 +-
>   gcc/config/alpha/alpha.cc           |  5 ++---
>   gcc/config/arm/aout.h               |  2 +-
>   gcc/config/arm/arm.cc               |  2 +-
>   gcc/config/bfin/bfin.h              | 16 ++++++++--------
>   gcc/config/c6x/c6x.h                |  2 +-
>   gcc/config/gcn/gcn.cc               |  5 ++---
>   gcc/config/h8300/h8300.h            |  2 +-
>   gcc/config/ia64/ia64.cc             |  5 ++---
>   gcc/config/mcore/mcore-elf.h        |  2 +-
>   gcc/config/microblaze/microblaze.cc |  3 +--
>   gcc/config/mips/mips.cc             | 19 ++++++++++---------
>   gcc/config/pa/pa.cc                 |  3 ++-
>   gcc/config/riscv/riscv.cc           |  2 +-
>   gcc/config/rs6000/rs6000.cc         |  4 ++--
>   15 files changed, 36 insertions(+), 38 deletions(-)
Essentially none of these ports define ASM_OUTPUT_FUCNTION_LABEL and the 
fallback just uses ASM_OUTPUT_LABEL under the hood.  So this should have 
no functional change.  OK for the trunk.

jeff
  
Jan-Benedict Glaw Jan. 12, 2024, 9:47 p.m. UTC | #2
On Tue, 2024-01-02 20:41:37 +0100, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> diff --git a/gcc/config/ia64/ia64.cc b/gcc/config/ia64/ia64.cc
> index ac566efcf19..92d00bf922f 100644
> --- a/gcc/config/ia64/ia64.cc
> +++ b/gcc/config/ia64/ia64.cc
> @@ -3886,8 +3886,7 @@ ia64_expand_prologue (void)
>  /* Output the textual info surrounding the prologue.  */
>  
>  void
> -ia64_start_function (FILE *file, const char *fnname,
> -		     tree decl ATTRIBUTE_UNUSED)
> +ia64_start_function (FILE *file, const char *fnname, tree decl)
>  {
>  #if TARGET_ABI_OPEN_VMS
>    vms_start_function (fnname);
> @@ -3896,7 +3895,7 @@ ia64_start_function (FILE *file, const char *fnname,
>    fputs ("\t.proc ", file);
>    assemble_name (file, fnname);
>    fputc ('\n', file);
> -  ASM_OUTPUT_LABEL (file, fnname);
> +  ASM_OUTPUT_FUNCTION_LABEL (file, fnname, decl);
>  }
>  
>  /* Called after register allocation to add any instructions needed for the

Seems for this I'll get a new warning (forced to error by configuring
with --enable-werror-always), cf. http://toolchain.lug-owl.de/laminar/log/gcc-ia64-elf/48 :

[all 2024-01-12 16:32:32] /var/lib/laminar/run/gcc-ia64-elf/48/local-toolchain-install/bin/g++  -fno-PIE -c   -g -O2   -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -Wconditionally-supported -Woverloaded-virtual -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common  -DHAVE_CONFIG_H -fno-PIE -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include  -I../../gcc/gcc/../libcpp/include -I../../gcc/gcc/../libcody  -I../../gcc/gcc/../libdecnumber -I../../gcc/gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/gcc/../libbacktrace   -o ia64.o -MT ia64.o -MMD -MP -MF ./.deps/ia64.TPo ../../gcc/gcc/config/ia64/ia64.cc
[all 2024-01-12 16:32:34] ../../gcc/gcc/config/ia64/ia64.cc: In function 'void ia64_start_function(FILE*, const char*, tree)':
[all 2024-01-12 16:32:34] ../../gcc/gcc/config/ia64/ia64.cc:3889:59: error: unused parameter 'decl' [-Werror=unused-parameter]
[all 2024-01-12 16:32:34]  3889 | ia64_start_function (FILE *file, const char *fnname, tree decl)
[all 2024-01-12 16:32:34]       |                                                      ~~~~~^~~~
[all 2024-01-12 16:32:49] cc1plus: all warnings being treated as errors
[all 2024-01-12 16:32:49] make[1]: *** [Makefile:2555: ia64.o] Error 1


So the ATTRIBUTE_UNUSED seems to be a good cover, or update the
ASM_OUTPUT_FUNCTION_LABEL macro to always "use" its last argument.

MfG, JBG

--
  

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 298477d88bb..e3c72f60d4e 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -24207,7 +24207,7 @@  aarch64_declare_function_name (FILE *stream, const char* name,
 
   /* Don't forget the type directive for ELF.  */
   ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
-  ASM_OUTPUT_LABEL (stream, name);
+  ASM_OUTPUT_FUNCTION_LABEL (stream, name, fndecl);
 
   cfun->machine->label_is_assembled = true;
 }
diff --git a/gcc/config/alpha/alpha.cc b/gcc/config/alpha/alpha.cc
index 6aa93783226..8118255e737 100644
--- a/gcc/config/alpha/alpha.cc
+++ b/gcc/config/alpha/alpha.cc
@@ -7986,8 +7986,7 @@  int num_source_filenames = 0;
 /* Output the textual info surrounding the prologue.  */
 
 void
-alpha_start_function (FILE *file, const char *fnname,
-		      tree decl ATTRIBUTE_UNUSED)
+alpha_start_function (FILE *file, const char *fnname, tree decl)
 {
   unsigned long imask, fmask;
   /* Complete stack size needed.  */
@@ -8052,7 +8051,7 @@  alpha_start_function (FILE *file, const char *fnname,
   if (TARGET_ABI_OPEN_VMS)
     strcat (entry_label, "..en");
 
-  ASM_OUTPUT_LABEL (file, entry_label);
+  ASM_OUTPUT_FUNCTION_LABEL (file, entry_label, decl);
   inside_function = TRUE;
 
   if (TARGET_ABI_OPEN_VMS)
diff --git a/gcc/config/arm/aout.h b/gcc/config/arm/aout.h
index 49896bb9620..380147aed7d 100644
--- a/gcc/config/arm/aout.h
+++ b/gcc/config/arm/aout.h
@@ -152,7 +152,7 @@ 
   do							\
     {							\
       ARM_DECLARE_FUNCTION_NAME (STREAM, NAME, DECL);   \
-      ASM_OUTPUT_LABEL (STREAM, NAME);			\
+      ASM_OUTPUT_FUNCTION_LABEL (STREAM, NAME, DECL);	\
     }							\
   while (0)
 #endif
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 0c0cb14a8a4..7ca607b3de1 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -21800,7 +21800,7 @@  arm_asm_declare_function_name (FILE *file, const char *name, tree decl)
   ARM_DECLARE_FUNCTION_NAME (file, name, decl);
   ASM_OUTPUT_TYPE_DIRECTIVE (file, name, "function");
   ASM_DECLARE_RESULT (file, DECL_RESULT (decl));
-  ASM_OUTPUT_LABEL (file, name);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 
   if (cmse_name)
     ASM_OUTPUT_LABEL (file, cmse_name);
diff --git a/gcc/config/bfin/bfin.h b/gcc/config/bfin/bfin.h
index c25f41f6839..60a8d716819 100644
--- a/gcc/config/bfin/bfin.h
+++ b/gcc/config/bfin/bfin.h
@@ -995,14 +995,14 @@  typedef enum directives {
         fputc ('\n',FILE);			\
       } while (0)
 
-#define ASM_DECLARE_FUNCTION_NAME(FILE,NAME,DECL) \
-  do {					\
-    fputs (".type ", FILE);           	\
-    assemble_name (FILE, NAME);         \
-    fputs (", STT_FUNC", FILE);         \
-    fputc (';',FILE);                   \
-    fputc ('\n',FILE);			\
-    ASM_OUTPUT_LABEL(FILE, NAME);	\
+#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL)	\
+  do {							\
+    fputs (".type ", FILE);				\
+    assemble_name (FILE, NAME);				\
+    fputs (", STT_FUNC", FILE);				\
+    fputc (';', FILE);					\
+    fputc ('\n', FILE);					\
+    ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);	\
   } while (0)
 
 #define ASM_OUTPUT_LABEL(FILE, NAME)    \
diff --git a/gcc/config/c6x/c6x.h b/gcc/config/c6x/c6x.h
index 26b2f2f0700..790b9627ebe 100644
--- a/gcc/config/c6x/c6x.h
+++ b/gcc/config/c6x/c6x.h
@@ -459,7 +459,7 @@  struct GTY(()) machine_function
       c6x_output_file_unwind (FILE);				\
       ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
       ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
-      ASM_OUTPUT_LABEL (FILE, NAME);				\
+      ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);		\
     }								\
   while (0)
 
diff --git a/gcc/config/gcn/gcn.cc b/gcc/config/gcn/gcn.cc
index b67551a2e8e..b2528d49de4 100644
--- a/gcc/config/gcn/gcn.cc
+++ b/gcc/config/gcn/gcn.cc
@@ -6555,7 +6555,7 @@  output_file_start (void)
    comments that pass information to mkoffload.  */
 
 void
-gcn_hsa_declare_function_name (FILE *file, const char *name, tree)
+gcn_hsa_declare_function_name (FILE *file, const char *name, tree decl)
 {
   int sgpr, vgpr, avgpr;
   bool xnack_enabled = TARGET_XNACK;
@@ -6716,8 +6716,7 @@  gcn_hsa_declare_function_name (FILE *file, const char *name, tree)
   fputs ("\t.type\t", file);
   assemble_name (file, name);
   fputs (",@function\n", file);
-  assemble_name (file, name);
-  fputs (":\n", file);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 
   /* This comment is read by mkoffload.  */
   if (flag_openacc)
diff --git a/gcc/config/h8300/h8300.h b/gcc/config/h8300/h8300.h
index 311e4023dfd..b62779a9273 100644
--- a/gcc/config/h8300/h8300.h
+++ b/gcc/config/h8300/h8300.h
@@ -650,7 +650,7 @@  struct cum_arg
 #define GLOBAL_ASM_OP "\t.global "
 
 #define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \
-   ASM_OUTPUT_LABEL (FILE, NAME)
+   ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL)
 
 /* This is how to store into the string LABEL
    the symbol_ref name of an internal numbered label where
diff --git a/gcc/config/ia64/ia64.cc b/gcc/config/ia64/ia64.cc
index ac566efcf19..92d00bf922f 100644
--- a/gcc/config/ia64/ia64.cc
+++ b/gcc/config/ia64/ia64.cc
@@ -3886,8 +3886,7 @@  ia64_expand_prologue (void)
 /* Output the textual info surrounding the prologue.  */
 
 void
-ia64_start_function (FILE *file, const char *fnname,
-		     tree decl ATTRIBUTE_UNUSED)
+ia64_start_function (FILE *file, const char *fnname, tree decl)
 {
 #if TARGET_ABI_OPEN_VMS
   vms_start_function (fnname);
@@ -3896,7 +3895,7 @@  ia64_start_function (FILE *file, const char *fnname,
   fputs ("\t.proc ", file);
   assemble_name (file, fnname);
   fputc ('\n', file);
-  ASM_OUTPUT_LABEL (file, fnname);
+  ASM_OUTPUT_FUNCTION_LABEL (file, fnname, decl);
 }
 
 /* Called after register allocation to add any instructions needed for the
diff --git a/gcc/config/mcore/mcore-elf.h b/gcc/config/mcore/mcore-elf.h
index bf1b093d327..3e0b903727b 100644
--- a/gcc/config/mcore/mcore-elf.h
+++ b/gcc/config/mcore/mcore-elf.h
@@ -51,7 +51,7 @@  along with GCC; see the file COPYING3.  If not see
 	}							\
       ASM_OUTPUT_TYPE_DIRECTIVE (FILE, NAME, "function");	\
       ASM_DECLARE_RESULT (FILE, DECL_RESULT (DECL));		\
-      ASM_OUTPUT_LABEL (FILE, NAME);				\
+      ASM_OUTPUT_FUNCTION_LABEL (FILE, NAME, DECL);		\
     }								\
   while (0)
 
diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc
index 3ea177b835e..79405a32dc1 100644
--- a/gcc/config/microblaze/microblaze.cc
+++ b/gcc/config/microblaze/microblaze.cc
@@ -2792,8 +2792,7 @@  microblaze_function_prologue (FILE * file)
 	ASM_OUTPUT_TYPE_DIRECTIVE (file, fnname, "function");
     }
 
-  assemble_name (file, fnname);
-  fputs (":\n", file);
+  ASM_OUTPUT_FUNCTION_LABEL (file, fnname, current_function_decl);
 
   if (interrupt_handler && strcmp (INTERRUPT_HANDLER_NAME, fnname))
     fputs ("_interrupt_handler:\n", file);
diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc
index 9180dbbf843..be5302b0b7f 100644
--- a/gcc/config/mips/mips.cc
+++ b/gcc/config/mips/mips.cc
@@ -7269,7 +7269,7 @@  mips_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p,
 /* Declare a unique, locally-binding function called NAME, then start
    its definition.  */
 
-static void
+static tree
 mips_start_unique_function (const char *name)
 {
   tree decl;
@@ -7291,13 +7291,15 @@  mips_start_unique_function (const char *name)
   fputs ("\t.hidden\t", asm_out_file);
   assemble_name (asm_out_file, name);
   putc ('\n', asm_out_file);
+
+  return decl;
 }
 
 /* Start a definition of function NAME.  MIPS16_P indicates whether the
    function contains MIPS16 code.  */
 
 static void
-mips_start_function_definition (const char *name, bool mips16_p)
+mips_start_function_definition (const char *name, bool mips16_p, tree decl)
 {
   if (mips16_p)
     fprintf (asm_out_file, "\t.set\tmips16\n");
@@ -7321,8 +7323,7 @@  mips_start_function_definition (const char *name, bool mips16_p)
   ASM_OUTPUT_TYPE_DIRECTIVE (asm_out_file, name, "function");
 
   /* Start the definition proper.  */
-  assemble_name (asm_out_file, name);
-  fputs (":\n", asm_out_file);
+  ASM_OUTPUT_FUNCTION_LABEL (asm_out_file, name, decl);
 }
 
 /* End a function definition started by mips_start_function_definition.  */
@@ -7349,8 +7350,8 @@  mips_finish_stub (mips_one_only_stub **stub_ptr)
     return;
 
   const char *name = stub->get_name ();
-  mips_start_unique_function (name);
-  mips_start_function_definition (name, false);
+  tree decl = mips_start_unique_function (name);
+  mips_start_function_definition (name, false, decl);
   stub->output_body ();
   mips_end_function_definition (name);
   delete stub;
@@ -7604,7 +7605,7 @@  mips16_build_function_stub (void)
 
   /* Start the function definition.  */
   assemble_start_function (stubdecl, stubname);
-  mips_start_function_definition (stubname, false);
+  mips_start_function_definition (stubname, false, stubdecl);
 
   /* If generating pic2 code, either set up the global pointer or
      switch to pic0.  */
@@ -7864,7 +7865,7 @@  mips16_build_call_stub (rtx retval, rtx *fn_ptr, rtx args_size, int fp_code)
 
       /* Start the function definition.  */
       assemble_start_function (stubdecl, stubname);
-      mips_start_function_definition (stubname, false);
+      mips_start_function_definition (stubname, false, stubdecl);
 
       if (fp_ret_p)
 	{
@@ -12064,7 +12065,7 @@  mips_output_function_prologue (FILE *file)
      assemble_start_function.  This is needed so that the name used here
      exactly matches the name used in ASM_DECLARE_FUNCTION_NAME.  */
   fnname = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
-  mips_start_function_definition (fnname, TARGET_MIPS16);
+  mips_start_function_definition (fnname, TARGET_MIPS16, current_function_decl);
 
   /* Output MIPS-specific frame information.  */
   if (!flag_inhibit_size_directive)
diff --git a/gcc/config/pa/pa.cc b/gcc/config/pa/pa.cc
index 2ee987796f6..cd9210a3ff9 100644
--- a/gcc/config/pa/pa.cc
+++ b/gcc/config/pa/pa.cc
@@ -3991,7 +3991,8 @@  pa_output_function_label (FILE *file)
   /* The function's label and associated .PROC must never be
      separated and must be output *after* any profiling declarations
      to avoid changing spaces/subspaces within a procedure.  */
-  ASM_OUTPUT_LABEL (file, XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0));
+  const char *name = XSTR (XEXP (DECL_RTL (current_function_decl), 0), 0);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, current_function_decl);
   fputs ("\t.PROC\n", file);
 
   /* pa_expand_prologue does the dirty work now.  We just need
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0d1cbc5cb5f..974f6d5f917 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -8486,7 +8486,7 @@  riscv_declare_function_name (FILE *stream, const char *name, tree fndecl)
 {
   riscv_asm_output_variant_cc (stream, fndecl, name);
   ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function");
-  ASM_OUTPUT_LABEL (stream, name);
+  ASM_OUTPUT_FUNCTION_LABEL (stream, name, fndecl);
   if (DECL_FUNCTION_SPECIFIC_TARGET (fndecl))
     {
       fprintf (stream, "\t.option push\n");
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 6b9a40fcc66..6c9f99c6e87 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -21419,7 +21419,7 @@  rs6000_elf_declare_function_name (FILE *file, const char *name, tree decl)
       fputs ("\t.long 0\n", file);
       fprintf (file, "\t.previous\n");
     }
-  ASM_OUTPUT_LABEL (file, name);
+  ASM_OUTPUT_FUNCTION_LABEL (file, name, decl);
 }
 
 static void rs6000_elf_file_end (void) ATTRIBUTE_UNUSED;
@@ -21992,7 +21992,7 @@  rs6000_xcoff_declare_function_name (FILE *file, const char *name, tree decl)
   assemble_name (file, buffer);
   fputs (TARGET_32BIT ? "\n" : ",3\n", file);
 
-  ASM_OUTPUT_LABEL (file, buffer);
+  ASM_OUTPUT_FUNCTION_LABEL (file, buffer, decl);
 
   symtab_node::get (decl)->call_for_symbol_and_aliases (rs6000_declare_alias,
 							&data, true);