Re: [PATCH v2 1/6] Gas changes for nanoMIPS support.

Jan Beulich <[email protected]> Fri, 31 Jul 2026 12:47:02 +0200
Newsgroups gmane.comp.gnu.binutils
Message-ID <[email protected]>
On 24.07.2026 12:30, Tsing wrote:
> From: tsing <[email protected]>
> 
> /
> 	* configure.ac: Add nanoMIPS target.
> 	* config.sub: Likewise.

I can't spot a change corresponding to this line.

> 	* configure: Regenerate.

Overall, changes to top-level files need to go to gcc first, I think, or
- in the case of config-sub - through the config project.

> config/
> 	* mt-nanomips: New file.
> gas/
> 	* Makefile.am: Add nanomips files.
> 	* Makefile.in: Regenerate.
> 	* config.in: Define a default architecture.
> 	* configure: Regenerate.
> 	* configure.ac: Likewise.
> 	* configure.tgt: Add entries for nanomips.
> 	* read.c: Likewise.
> 	* config/obj-elf.h: Likewise.
> 	* config/tc-nanomips.c: New file.
> 	* config/tc-nanomips.h: New file.
> ld/
> 	* configure.tgt: Add nanomips entries.

Isn't this patch about gas?

> ---
>  config/mt-nanomips       |     3 +
>  configure                |     9 +
>  configure.ac             |     9 +
>  gas/Makefile.am          |     2 +
>  gas/Makefile.in          |     5 +
>  gas/config.in            |     9 +
>  gas/config/obj-elf.h     |     5 +
>  gas/config/tc-nanomips.c | 11667 +++++++++++++++++++++++++++++++++++++
>  gas/config/tc-nanomips.h |   187 +

Now this is concerning. Of course I know nothing about nanoMIPS and next to
nothing about MIPS, but are these two really so different that almost 12k
lines of new code are warranted?

> --- /dev/null
> +++ b/config/mt-nanomips
> @@ -0,0 +1,3 @@
> +# We default to building libraries optimised for size.
> +CFLAGS_FOR_TARGET += -Os -mno-gpopt
> +CXXFLAGS_FOR_TARGET += -Os -mno-gpopt

This looks wrong to me: Why would this be enforced onto everyone building
for this target? (For -Os we already have config/mt-ospace.) I realize
there are pre-existing examples, but those not targeted at particular OSes
look at least questionable to me.

> --- a/gas/Makefile.am
> +++ b/gas/Makefile.am
> @@ -179,6 +179,7 @@ TARGET_CPU_CFILES = \
>  	config/tc-metag.c \
>  	config/tc-microblaze.c \
>  	config/tc-mips.c \
> +	config/tc-nanomips.c \
>  	config/tc-mmix.c \
>  	config/tc-mn10200.c \
>  	config/tc-mn10300.c \
> @@ -253,6 +254,7 @@ TARGET_CPU_HFILES = \
>  	config/tc-metag.h \
>  	config/tc-microblaze.h \
>  	config/tc-mips.h \
> +	config/tc-nanomips.h \
>  	config/tc-mmix.h \
>  	config/tc-mn10200.h \
>  	config/tc-mn10300.h \

Please insert alphabetically correct.

> --- a/gas/config/obj-elf.h
> +++ b/gas/config/obj-elf.h
> @@ -49,6 +49,11 @@ extern int alpha_flag_mdebug;
>  extern int mips_flag_mdebug;
>  #endif /* TC_MIPS */
>  
> +/* Always disable ECOFF_DEBUGGING for nanoMIPS target.  */
> +#ifdef TC_NANOMIPS
> +#define ECOFF_DEBUGGING 0
> +#endif /* TC_NANOMIPS */

Why would this be needed? You enable only ELF afaics.

> --- a/gas/read.c
> +++ b/gas/read.c
> @@ -2615,7 +2615,7 @@ bss_alloc (symbolS *symbolP, addressT size, unsigned int align)
>    subsegT current_subseg = now_subseg;
>    segT bss_seg = bss_section;
>  
> -#if defined (TC_MIPS) || defined (TC_ALPHA)
> +#if defined (TC_MIPS) || defined (TC_ALPHA) || defined (TC_NANOMIPS)
>    if (OUTPUT_FLAVOR == bfd_target_ecoff_flavour
>        || OUTPUT_FLAVOR == bfd_target_elf_flavour)
>      {
Hmm, I'm not very happy to see such a hack being further extended.

Jan