Evgeny Karpov <evgeny.kar...@microsoft.com> writes:
> LOCAL_LABEL_PREFIX has been changed to help the assembly
> compiler recognize local labels. Emitting locals has been
> replaced with the .lcomm directive to declare uninitialized
> data without defining an exact section. Functions and objects
> were missing declarations. Binutils was not able to distinguish
> static from external, or an object from a function.
> mingw_pe_declare_object_type has been added to have type
> information for relocation on AArch64, which is not the case
> for ix86.
>
> This fix relies on changes in binutils.
> aarch64: Relocation fixes and LTO
> https://sourceware.org/pipermail/binutils/2024-August/136481.html
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64-coff.h (LOCAL_LABEL_PREFIX):
> Use "." as the local label prefix.
> (ASM_OUTPUT_ALIGNED_LOCAL): Remove.
> (ASM_OUTPUT_LOCAL): New.
> * config/aarch64/cygming.h (ASM_DECLARE_OBJECT_NAME):
> New.
> (ASM_DECLARE_FUNCTION_NAME): New.
> * config/mingw/winnt.cc (mingw_pe_declare_object_type):
> New.
> * config/mingw/winnt.h (mingw_pe_declare_object_type):
> New.
> ---
> gcc/config/aarch64/aarch64-coff.h | 22 ++++++----------------
> gcc/config/aarch64/cygming.h | 12 ++++++++++++
> gcc/config/mingw/winnt.cc | 10 ++++++++++
> gcc/config/mingw/winnt.h | 2 ++
> 4 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-coff.h
> b/gcc/config/aarch64/aarch64-coff.h
> index 81fd9954f75..77c09df82e4 100644
> --- a/gcc/config/aarch64/aarch64-coff.h
> +++ b/gcc/config/aarch64/aarch64-coff.h
> @@ -20,9 +20,8 @@
> #ifndef GCC_AARCH64_COFF_H
> #define GCC_AARCH64_COFF_H
>
> -#ifndef LOCAL_LABEL_PREFIX
> -# define LOCAL_LABEL_PREFIX ""
> -#endif
> +#undef LOCAL_LABEL_PREFIX
> +#define LOCAL_LABEL_PREFIX "."
>
> /* Using long long breaks -ansi and -std=c90, so these will need to be
> made conditional for an LLP64 ABI. */
> @@ -54,19 +53,10 @@
> }
> #endif
>
> -/* Output a local common block. /bin/as can't do this, so hack a
> - `.space' into the bss segment. Note that this is *bad* practice,
> - which is guaranteed NOT to work since it doesn't define STATIC
> - COMMON space but merely STATIC BSS space. */
> -#ifndef ASM_OUTPUT_ALIGNED_LOCAL
> -# define ASM_OUTPUT_ALIGNED_LOCAL(STREAM, NAME, SIZE, ALIGN) \
> - {
> \
> - switch_to_section (bss_section);
> \
> - ASM_OUTPUT_ALIGN (STREAM, floor_log2 (ALIGN / BITS_PER_UNIT)); \
> - ASM_OUTPUT_LABEL (STREAM, NAME);
> \
> - fprintf (STREAM, "\t.space\t%d\n", (int)(SIZE));
> \
> - }
> -#endif
> +#define ASM_OUTPUT_LOCAL(FILE, NAME, SIZE, ROUNDED) \
> +( fputs (".lcomm ", (FILE)), \
> + assemble_name ((FILE), (NAME)), \
> + fprintf ((FILE), ",%u\n", (int)(ROUNDED)))
I realise this is pre-existing, bue the last line should probably be:
fprintf ((FILE), "," HOST_WIDE_INT_PRINT_UNSIGNED "\n", (ROUNDED)))
to avoid silent truncation. (Even if the format only supports 32-bit
code and data, it's better for out-of-bounds values to be flagged by
the assembler rather than silently truncated.)
> #define ASM_OUTPUT_SKIP(STREAM, NBYTES) \
> fprintf (STREAM, "\t.space\t%d // skip\n", (int) (NBYTES))
> diff --git a/gcc/config/aarch64/cygming.h b/gcc/config/aarch64/cygming.h
> index e4ceab82b9e..d3c6f550b68 100644
> --- a/gcc/config/aarch64/cygming.h
> +++ b/gcc/config/aarch64/cygming.h
> @@ -213,6 +213,18 @@ still needed for compilation. */
>
> #define SUPPORTS_ONE_ONLY 1
>
> +#undef ASM_DECLARE_OBJECT_NAME
> +#define ASM_DECLARE_OBJECT_NAME(STREAM, NAME, DECL) \
> + mingw_pe_declare_object_type (STREAM, NAME, TREE_PUBLIC (DECL)); \
> + ASM_OUTPUT_LABEL ((STREAM), (NAME))
> +
> +
> +#undef ASM_DECLARE_FUNCTION_NAME
> +#define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL) \
> + mingw_pe_declare_function_type (STR, NAME, TREE_PUBLIC (DECL)); \
> + aarch64_declare_function_name (STR, NAME, DECL)
> +
> +
These two should probaly either be wrapped in:
do { ... ] while (0)
or use comma expressions (as for the .lcomm printing above).
Using "STREAM" rather than "STR" in ASM_DECLARE_FUNCTION_NAME
would be more consistent with ASM_DECLARE_OBJECT_NAME.
> /* Define this to be nonzero if static stack checking is supported. */
> #define STACK_CHECK_STATIC_BUILTIN 1
>
> diff --git a/gcc/config/mingw/winnt.cc b/gcc/config/mingw/winnt.cc
> index 1e2ec53e841..64157b09644 100644
> --- a/gcc/config/mingw/winnt.cc
> +++ b/gcc/config/mingw/winnt.cc
> @@ -581,6 +581,16 @@ i386_pe_asm_output_aligned_decl_common (FILE *stream,
> tree decl,
> function, and PUB is nonzero if the function is globally
> visible. */
>
> +void
> +mingw_pe_declare_object_type (FILE *file, const char *name, int pub)
The new function should have its own comment (the existing one
describes mingw_pe_declare_function_type). Could we make "pub"
a bool for both functions?
Maybe the two functions are similar enough that it would be worth
having them forward to an internal helper that takes DT_NON or DT_FCN
as appropriate. I suppose that's more personal preference though,
so let me know if you disagree.
Looks good otherwise.
Thanks,
Richard
> +{
> + fprintf (file, "\t.def\t");
> + assemble_name (file, name);
> + fprintf (file, ";\t.scl\t%d;\t.type\t%d;\t.endef\n",
> + pub ? (int) C_EXT : (int) C_STAT,
> + (int) DT_NON << N_BTSHFT);
> +}
> +
> void
> mingw_pe_declare_function_type (FILE *file, const char *name, int pub)
> {
> diff --git a/gcc/config/mingw/winnt.h b/gcc/config/mingw/winnt.h
> index a21a36b7e5d..f375d071170 100644
> --- a/gcc/config/mingw/winnt.h
> +++ b/gcc/config/mingw/winnt.h
> @@ -25,6 +25,8 @@ extern tree mingw_handle_selectany_attribute (tree *, tree,
> tree, int, bool *);
> extern void mingw_pe_asm_named_section (const char *, unsigned int, tree);
> extern void mingw_pe_declare_function_type (FILE *file, const char *name,
> int pub);
> +extern void mingw_pe_declare_object_type (FILE *file, const char *name,
> + int pub);
> extern void mingw_pe_encode_section_info (tree, rtx, int);
> extern void mingw_pe_file_end (void);
> extern void mingw_pe_maybe_record_exported_symbol (tree, const char *, int);
