diff mbox

[avr] : Fix various problems with specs and specs file generation.

Message ID 54F4BCA1.7080707@gjlay.de
State New
Headers show

Commit Message

Georg-Johann Lay March 2, 2015, 7:40 p.m. UTC
The new specs file generation introduces several problems.  This patch

- Fix build warnings

- Clean up unused code and the old, now dead specs definitions.

- Removes unused files and adjust build scripts / rules.

- Issues with avr-libc awareness:

-- Makes specs-generation aware of avr-libc (include tm.h in build script and 
depend on WITH_AVRLIBC).

-- avr-libc implements functions in libm.a which usually live in libgcc, hence 
add -lm to libgcc_spec, cf. PR54461.

-- The new libdev.a is a feature of avr-libc and not available with, e.g., 
newlib.  Hence remove it from lib_spec if the compiler is not configured for 
avr-libc (--with-avrlibc=no).

- Many minor issues with option propagation.

- -march had been added to multilib generation some time ago, but 
driver_self_spec was not aware of that option.


Ok for trunk?


BTW, anyone knows what -march= is good for?  It allows all kinds of silly 
option combinations like "-march=avrtiny -mmcu=atmega8" without any complaints.

What's wrong with -mmcu=avr* ? All architecture names start with "avr", and 
/no/ device name starts /with/ "avr", hence -mmcu= should be fine and work as 
smooth as in the last 10 years.

Why that -march= option?  If there is not a good reason for -march=, I'd 
propose to clean it up and use -mmcu=avr* instead.


Johann


gcc/
	Fix various problems with specs file generation.

	* config.gcc (extra_gcc_objs) [avr]: Remove.

	* config/avr/driver-avr.c: Remove file.
	* config/avr/t-avr (driver-avr.o): Remove rule.
	(gen-avr-mmcu-specs): Use CXX_FOR_BUILD, CXXFLAGS_FOR_BUILD and
	INCLUDES to build.  Depend on TM_H.
	* config/avr/gen-avr-mmcu-specs.c: Tidy up code.  Fix various
	build warnings.  Fix non-matching types and non-existing %-codes.
	(tm.h): Include.
	(*lib) [!WITH_AVRLIBC]: Don't link libdev.a.
	(*libgcc) [WITH_AVRLIBC]: Add "-lm".
	* config/avr/avrlibc.h (LIBGCC_SPEC): Remove definition.
	* config/avr/avr.h (DRIVER_SELF_SPECS): Fix handling of -march=.
	(CPP_SPEC, CC1PLUS_SPEC, ASM_SPEC, LINK_SPEC, LIB_SPEC)
	(LIBGCC_SPEC): Remove definitions.

Comments

Denis Chertykov March 3, 2015, 7:46 a.m. UTC | #1
2015-03-02 22:40 GMT+03:00 Georg-Johann Lay <avr@gjlay.de>:
> The new specs file generation introduces several problems.  This patch
>
> - Fix build warnings
>
> - Clean up unused code and the old, now dead specs definitions.
>
> - Removes unused files and adjust build scripts / rules.
>
> - Issues with avr-libc awareness:
>
> -- Makes specs-generation aware of avr-libc (include tm.h in build script
> and depend on WITH_AVRLIBC).
>
> -- avr-libc implements functions in libm.a which usually live in libgcc,
> hence add -lm to libgcc_spec, cf. PR54461.
>
> -- The new libdev.a is a feature of avr-libc and not available with, e.g.,
> newlib.  Hence remove it from lib_spec if the compiler is not configured for
> avr-libc (--with-avrlibc=no).
>
> - Many minor issues with option propagation.
>
> - -march had been added to multilib generation some time ago, but
> driver_self_spec was not aware of that option.
>
>
> Ok for trunk?

Approved.

>
>
> BTW, anyone knows what -march= is good for?  It allows all kinds of silly
> option combinations like "-march=avrtiny -mmcu=atmega8" without any
> complaints.
>
> What's wrong with -mmcu=avr* ? All architecture names start with "avr", and
> /no/ device name starts /with/ "avr", hence -mmcu= should be fine and work
> as smooth as in the last 10 years.
>
> Why that -march= option?  If there is not a good reason for -march=, I'd
> propose to clean it up and use -mmcu=avr* instead.


I do not remember.
I guess that it's like -march / -mcpu in ARM target.


Denis.
Senthil Kumar Selvaraj March 3, 2015, 1:21 p.m. UTC | #2
On Mon, Mar 02, 2015 at 08:40:17PM +0100, Georg-Johann Lay wrote:
> The new specs file generation introduces several problems.  This patch
> 
> - Fix build warnings
> 
> - Clean up unused code and the old, now dead specs definitions.
> 
> - Removes unused files and adjust build scripts / rules.
> 
> - Issues with avr-libc awareness:
> 
> -- Makes specs-generation aware of avr-libc (include tm.h in build script
> and depend on WITH_AVRLIBC).
> 
> -- avr-libc implements functions in libm.a which usually live in libgcc,
> hence add -lm to libgcc_spec, cf. PR54461.
> 
> -- The new libdev.a is a feature of avr-libc and not available with, e.g.,
> newlib.  Hence remove it from lib_spec if the compiler is not configured for
> avr-libc (--with-avrlibc=no).
> 
> - Many minor issues with option propagation.
> 
> - -march had been added to multilib generation some time ago, but
> driver_self_spec was not aware of that option.
> 
> 
> Ok for trunk?
> 
> 
> BTW, anyone knows what -march= is good for?  It allows all kinds of silly
> option combinations like "-march=avrtiny -mmcu=atmega8" without any
> complaints.

IIRC, -march was added because replacing -mmcu=<device> to
-mmcu=<arch> in the driver's self specs broke multilib selection - the
driver always acted as if no -mmcu was specified. Adding
a new option (-march), translating mmcu=<device> to march=<arch> and then 
basing t-multilib on that worked ok.

> 
> What's wrong with -mmcu=avr* ? All architecture names start with "avr", and
> /no/ device name starts /with/ "avr", hence -mmcu= should be fine and work
> as smooth as in the last 10 years.
> 
> Why that -march= option?  If there is not a good reason for -march=, I'd
> propose to clean it up and use -mmcu=avr* instead.
> 
> 
> Johann
> 
> 
> gcc/
> 	Fix various problems with specs file generation.
> 
> 	* config.gcc (extra_gcc_objs) [avr]: Remove.
> 
> 	* config/avr/driver-avr.c: Remove file.
> 	* config/avr/t-avr (driver-avr.o): Remove rule.
> 	(gen-avr-mmcu-specs): Use CXX_FOR_BUILD, CXXFLAGS_FOR_BUILD and
> 	INCLUDES to build.  Depend on TM_H.
> 	* config/avr/gen-avr-mmcu-specs.c: Tidy up code.  Fix various
> 	build warnings.  Fix non-matching types and non-existing %-codes.
> 	(tm.h): Include.
> 	(*lib) [!WITH_AVRLIBC]: Don't link libdev.a.
> 	(*libgcc) [WITH_AVRLIBC]: Add "-lm".
> 	* config/avr/avrlibc.h (LIBGCC_SPEC): Remove definition.
> 	* config/avr/avr.h (DRIVER_SELF_SPECS): Fix handling of -march=.
> 	(CPP_SPEC, CC1PLUS_SPEC, ASM_SPEC, LINK_SPEC, LIB_SPEC)
> 	(LIBGCC_SPEC): Remove definitions.
> 

> Index: config.gcc
> ===================================================================
> --- config.gcc	(revision 220854)
> +++ config.gcc	(working copy)
> @@ -1103,7 +1103,6 @@ avr-*-*)
>  	fi
>  	tmake_file="${tmake_file} avr/t-avr avr/t-multilib"
>  	use_gcc_stdint=wrap
> -	extra_gcc_objs="driver-avr.o avr-devices.o"
>  	extra_objs="avr-devices.o avr-log.o"
>  	;;
>  bfin*-elf*)
> Index: config/avr/t-avr
> ===================================================================
> --- config/avr/t-avr	(revision 221028)
> +++ config/avr/t-avr	(working copy)
> @@ -16,10 +16,6 @@
>  # along with GCC; see the file COPYING3.  If not see
>  # <http://www.gnu.org/licenses/>.
>  
> -driver-avr.o: $(srcdir)/config/avr/driver-avr.c \
> -  $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H)
> -	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) $<
> -
>  avr-devices.o: $(srcdir)/config/avr/avr-devices.c \
>    $(srcdir)/config/avr/avr-mcus.def \
>    $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H)
> @@ -69,8 +65,8 @@ gen-avr-mmcu-texi$(build_exeext): $(srcd
>  
>  gen-avr-mmcu-specs$(build_exeext): $(srcdir)/config/avr/gen-avr-mmcu-specs.c \
>    $(AVR_MCUS) $(srcdir)/config/avr/avr-devices.c \
> -  $(srcdir)/config/avr/avr-arch.h
> -	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $< -o $@
> +  $(srcdir)/config/avr/avr-arch.h $(TM_H)
> +	$(CXX_FOR_BUILD) $(CXXFLAGS_FOR_BUILD) $< -o $@ $(INCLUDES)
>  
>  $(srcdir)/doc/avr-mmcu.texi: gen-avr-mmcu-texi$(build_exeext)
>  	$(RUN_GEN) ./$< > $@
> Index: config/avr/gen-avr-mmcu-specs.c
> ===================================================================
> --- config/avr/gen-avr-mmcu-specs.c	(revision 221028)
> +++ config/avr/gen-avr-mmcu-specs.c	(working copy)
> @@ -26,9 +26,30 @@
>  #include "avr-arch.h"
>  #include "avr-devices.c"
>  
> +#define GCC_DEFAULTS_H
> +
> +#include "tm.h"
> +
> +#if defined (WITH_AVRLIBC)
> +static const bool with_avrlibc = true;
> +#else
> +static const bool with_avrlibc = false;
> +#endif /* WITH_AVRLIBC */
> +
> +
> +/* Return true iff STR starts with PREFIX.  */
> +
> +static bool
> +str_prefix_p (const char *str, const char *prefix)
> +{
> +  return 0 == strncmp (str, prefix, strlen (prefix));
> +}
> +
> +
>  static void
>  print_mcu (const avr_mcu_t *mcu)
>  {
> +  const char *sp8_spec;
>    const avr_mcu_t *arch_mcu;
>  
>    for (arch_mcu = mcu; arch_mcu->macro; )
> @@ -37,100 +58,121 @@ for (arch_mcu = mcu; arch_mcu->macro; )
>      exit (EXIT_FAILURE);
>  
>    char name[100];
> -  if (snprintf (name, sizeof name, "specs-%s", mcu->name) >= sizeof name)
> +  if (snprintf (name, sizeof name, "specs-%s", mcu->name) >= (int) sizeof name)
>     exit (EXIT_FAILURE);
>  
>    FILE *f = fopen (name ,"w");
>  
> -  const char *sp8, *errata_skip, *rmw;
> -  /* Leave "avr2" and "avr25" alone.  These two architectures are
> -     the only ones that mix devices with 8-bit SP and 16-bit SP.  */
> +  bool errata_skip = 0 != (mcu->dev_attribute & AVR_ERRATA_SKIP);
> +  bool rmw = 0 != (mcu->dev_attribute & AVR_ISA_RMW);
> +  bool sp8 = 0 != (mcu->dev_attribute & AVR_SHORT_SP);
> +
>    if (mcu->macro == NULL
>        && (mcu->arch == ARCH_AVR2 || mcu->arch == ARCH_AVR25))
> -    sp8 = "";
> -
> -  sp8 = ((mcu->dev_attribute & AVR_SHORT_SP)
> -	 ? " -msp8" : " %<msp8");
> +    {
> +      // Leave "avr2" and "avr25" alone.  These two architectures are
> +      // the only ones that mix devices with 8-bit SP and 16-bit SP.
> +      sp8_spec = "";
> +    }
> +  else
> +    {
> +      sp8_spec = sp8
> +        ? " -msp8"
> +        : " %<msp8";
> +    }
>  
> -  errata_skip = (mcu->dev_attribute & AVR_ERRATA_SKIP) ? " -mskip-bug" : "";
> -  rmw = (mcu->dev_attribute & AVR_ISA_RMW) ? "%{!mno-rmw: -mrmw}" : "";
> +  const char *errata_skip_spec = errata_skip
> +    ? " %{!mno-skip-bug:-mskip-bug}"
> +    : " %{!mskip-bug:-mno-skip-bug}";
> +
> +  const char *rmw_spec = rmw
> +    ? " %{!mno-rmw: -mrmw}"
> +    : " %{mrmw}";
>  
>    const char *arch_name = avr_arch_types[mcu->arch].arch_name;
>  
> -  fprintf (f, "*self_spec:\n%%{!march=*:-march=%s}%s\n\n", arch_name, sp8);
> +  fprintf (f, "*self_spec:\n"
> +           " %%{!march=*:-march=%s}"
> +           " %s\n\n", arch_name, sp8_spec);
>  
>    if (mcu->macro)
>      fprintf (f, "*cpp:\n-D__AVR_DEV_LIB_NAME__=%s -D%s "
>  	     "-D__AVR_DEVICE_NAME__=%s\n\n",
>  	     mcu->library_name, mcu->macro, mcu->name);
>  
> -  fprintf (f, "*cc1:\n%s%s", errata_skip, rmw);
> +  fprintf (f, "*cc1:\n%s%s", errata_skip_spec, rmw_spec);
>    if (mcu->n_flash != arch_mcu->n_flash)
>      fprintf (f, " %%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
>    fprintf (f, "\n\n");
> -  fprintf (f, "*cc1plus:\n%s%s ", errata_skip, rmw);
> +
> +  fprintf (f, "*cc1plus:\n%s%s ", errata_skip_spec, rmw_spec);
>    if (mcu->n_flash != arch_mcu->n_flash)
> -    fprintf (f, "%%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
> -  fprintf (f, "%%{!frtti: -fno-rtti}"
> -	   "%%{!fenforce-eh-specs: -fno-enforce-eh-specs}"
> -	   "%%{!fexceptions: -fno-exceptions}\n\n");
> -
> -  fprintf (f, "*asm:\n%%{march=*:-mmcu=%%*}%{mrelax: --mlink-relax}%s\n\n",
> -	   *errata_skip ? "" : " -mno-skip-bug");
> -
> -  fprintf (f, "*link:\n%%{mrelax:--relax");
> -  if (strncmp (mcu->name, "at90usb8", strlen ("at90usb8")) == 0)
> -    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=8k}");
> -  if (strncmp (mcu->name, "atmega16", strlen ("atmega16")) == 0)
> -    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=16k}");
> -  if (strncmp (mcu->name, "atmega32", strlen ("atmega32")) == 0
> -      || strncmp (mcu->name, "at90can32", strlen ("at90can32")) == 0)
> -    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=32k}");
> -  if (strncmp (mcu->name, "atmega64", strlen ("atmega64")) == 0
> -      || strncmp (mcu->name, "at90can64", strlen ("at90can64")) == 0
> -      || strncmp (mcu->name, "at90usb64", strlen ("at90usb64")) == 0)
> -    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=64k}");
> -  fprintf (f, "} %%{march=*:-m%%*}");
> +    fprintf (f, " %%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
> +  fprintf (f, (" %%{!frtti: -fno-rtti}"
> +               " %%{!fenforce-eh-specs: -fno-enforce-eh-specs}"
> +               " %%{!fexceptions: -fno-exceptions}\n\n"));
> +
> +  fprintf (f, "*asm:\n"
> +           " %%{march=*:-mmcu=%%*}"
> +           " %%{mrelax: --mlink-relax}"
> +           " %s%s\n\n", rmw_spec, (errata_skip
> +                                  ? " %{mno-skip-bug}"
> +                                  : " %{!mskip-bug:-mno-skip-bug}"));
> +  fprintf (f, "*link:\n"
> +           " %%{mrelax:--relax");
> +  {
> +    int wrap_k =
> +      str_prefix_p (mcu->name, "at90usb8") ? 8
> +      : str_prefix_p (mcu->name, "atmega16") ? 16
> +      : (str_prefix_p (mcu->name, "atmega32")
> +         || str_prefix_p (mcu->name, "at90can32")) ? 32
> +      : (str_prefix_p (mcu->name, "atmega64")
> +        || str_prefix_p (mcu->name, "at90can64")
> +        || str_prefix_p (mcu->name, "at90usb64")) ? 64
> +      : 0;
> +
> +    if (wrap_k)
> +      fprintf (f, " %%{mpmem-wrap-around: --pmem-wrap-around=%dk}", wrap_k);
> +  }
> +  fprintf (f, "}"
> +           " %%{march=*:-m%%*}");
> +
>    if (mcu->data_section_start
>        != avr_arch_types[mcu->arch].default_data_section_start)
>      fprintf (f, " -Tdata 0x%lX", 0x800000UL + mcu->data_section_start);
> +
>    if (mcu->text_section_start != 0x0)
> -    fprintf (f, " -Ttext 0x%lX", mcu->text_section_start);
> +    fprintf (f, " -Ttext 0x%lX", 0UL + mcu->text_section_start);
>  
>    fprintf (f, " %%{shared:%%eshared is not supported}\n\n");
>  
> +  bool has_libs = mcu->arch != ARCH_AVR1;
> +
>    fprintf (f, "*lib:\n");
> -  if (strncmp (mcu->name, "mmcu=at90s1", strlen ("mmcu=at90s1")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny11", strlen ("mmcu=attiny11")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny12", strlen ("mmcu=attiny12")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny15", strlen ("mmcu=attiny15")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny28", strlen ("mmcu=attiny28")) != 0)
> +  if (has_libs)
>      {
>        fprintf (f, "-lc");
> -      if (mcu->macro)
> +      if (with_avrlibc
> +          && mcu->macro)
>  	fprintf (f, " dev/%s/libdev.a%%s", mcu->name);
>      }
>    fprintf (f, "\n\n");
>  
>    fprintf (f, "*libgcc:\n");
> -  if (strncmp (mcu->name, "mmcu=at90s1", strlen ("mmcu=at90s1")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny11", strlen ("mmcu=attiny11")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny12", strlen ("mmcu=attiny12")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny15", strlen ("mmcu=attiny15")) != 0
> -      && strncmp (mcu->name, "mmcu=attiny28", strlen ("mmcu=attiny28")) != 0)
> -    fprintf (f, "-lgcc");
> +  if (has_libs)
> +    fprintf (f, with_avrlibc
> +             ? "-lgcc -lm"
> +             : "-lgcc");
>    fprintf (f, "\n\n");
>  
> -  fprintf (f, "*startfile:\ndev/%s/crt1.o%%s\n\n", mcu->name);
> +  fprintf (f, "*startfile:\n"
> +           "dev/%s/crt1.o%%s\n\n", mcu->name);
>  }
>  
> +
>  int main (void)
>  {
> -  enum avr_arch arch = ARCH_UNKNOWN;
> -  size_t i, n_mcus = 0;
> -  const avr_mcu_t *mcu;
> -
> -  for (mcu = avr_mcu_types; mcu->name; mcu++)
> +  for (const avr_mcu_t *mcu = avr_mcu_types; mcu->name; mcu++)
>      print_mcu (mcu);
>  
>    return EXIT_SUCCESS;
> Index: config/avr/driver-avr.c
> ===================================================================
> --- config/avr/driver-avr.c	(revision 221028)
> +++ config/avr/driver-avr.c	(working copy)
> @@ -1,30 +0,0 @@
> -/* Subroutines for the gcc driver.
> -   Copyright (C) 2009-2015 Free Software Foundation, Inc.
> -   Contributed by Anatoly Sokolov <aesok@post.ru>
> -
> -This file is part of GCC.
> -
> -GCC is free software; you can redistribute it and/or modify
> -it under the terms of the GNU General Public License as published by
> -the Free Software Foundation; either version 3, or (at your option)
> -any later version.
> -
> -GCC is distributed in the hope that it will be useful,
> -but WITHOUT ANY WARRANTY; without even the implied warranty of
> -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> -GNU General Public License for more details.
> -
> -You should have received a copy of the GNU General Public License
> -along with GCC; see the file COPYING3.  If not see
> -<http://www.gnu.org/licenses/>.  */
> -
> -#include "config.h"
> -#include "system.h"
> -#include "coretypes.h"
> -#include "tm.h"
> -
> -/* Current architecture.  */
> -const avr_arch_t *avr_current_arch = NULL;
> -
> -/* Current device.  */
> -const avr_mcu_t *avr_current_device = NULL;
> Index: config/avr/avrlibc.h
> ===================================================================
> --- config/avr/avrlibc.h	(revision 221028)
> +++ config/avr/avrlibc.h	(working copy)
> @@ -19,11 +19,13 @@ You should have received a copy of the G
>  along with GCC; see the file COPYING3.  If not see
>  <http://www.gnu.org/licenses/>.  */
>  
> -/* AVR-Libc implements functions from libgcc.a in libm.a, see PR54461.  */
> +/* AVR-Libc implements functions from libgcc.a in libm.a, see PR54461.
> +   More AVR-Libc specific specs originate from gen-avr-mmcu-specs.c:
>  
> -#undef  LIBGCC_SPEC
> -#define LIBGCC_SPEC                                                     \
> -  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lgcc -lm }}}}}"
> +   - LIBGCC_SPEC (*libgcc)
> +   - LIB_SPEC (*lib)
> +   
> +*/
>  
>  #undef  LINK_GCC_C_SEQUENCE_SPEC
>  #define LINK_GCC_C_SEQUENCE_SPEC \
> Index: config/avr/avr.h
> ===================================================================
> --- config/avr/avr.h	(revision 221028)
> +++ config/avr/avr.h	(working copy)
> @@ -492,42 +492,18 @@ typedef struct avr_args
>  #define ADJUST_INSN_LENGTH(INSN, LENGTH)                \
>      (LENGTH = avr_adjust_insn_length (INSN, LENGTH))
>  
> -#define DRIVER_SELF_SPECS " %{mmcu=*:-specs=device-specs/specs-%*%s %<mmcu=*} "
> -#define CPP_SPEC ""
> +#define DRIVER_SELF_SPECS                                       \
> +  " %{!mmcu=*:%{!march=*:-specs=device-specs/specs-avr2%s} "    \
> +  "           %{march=*:-specs=device-specs/specs-%*%s}} "      \
> +  " %{mmcu=*:-specs=device-specs/specs-%*%s %<mmcu=*} "
>  
>  /* We want cc1plus used as a preprocessor to pick up the cpp spec from the
>     per-device spec files  */
>  #define CPLUSPLUS_CPP_SPEC "%(cpp)"
>  
> -#define CC1_SPEC ""
> -
> -#define CC1PLUS_SPEC "%{!frtti:-fno-rtti} \
> -    %{!fenforce-eh-specs:-fno-enforce-eh-specs} \
> -    %{!fexceptions:-fno-exceptions}"
> -
> -#define ASM_SPEC "%{march=*:-mmcu=%*}%{mrelax: --mlink-relax}"
> -  
> -#define LINK_SPEC "\
> -%{mrelax:--relax\
> -         %{mpmem-wrap-around:%{mmcu=at90usb8*:--pmem-wrap-around=8k}\
> -                             %{mmcu=atmega16*:--pmem-wrap-around=16k}\
> -                             %{mmcu=atmega32*|\
> -                               mmcu=at90can32*:--pmem-wrap-around=32k}\
> -                             %{mmcu=atmega64*|\
> -                               mmcu=at90can64*|\
> -                               mmcu=at90usb64*:--pmem-wrap-around=64k}}}\
> -%{march=*:-m%*}\
> -%{shared:%eshared is not supported}"
> -
> -#define LIB_SPEC \
> -  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lc }}}}}"
> -
>  #define LIBSTDCXX "gcc"
>  /* No libstdc++ for now.  Empty string doesn't work.  */
>  
> -#define LIBGCC_SPEC \
> -  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lgcc }}}}}"
> -
>  /* The actual definition will come from the device-specific spec file.  */
>  #define STARTFILE_SPEC ""
>
Georg-Johann Lay March 9, 2015, 4:21 p.m. UTC | #3
Am 03/03/2015 um 02:21 PM schrieb Senthil Kumar Selvaraj:
> On Mon, Mar 02, 2015 at 08:40:17PM +0100, Georg-Johann Lay wrote:
>> BTW, anyone knows what -march= is good for?  It allows all kinds of silly
>> option combinations like "-march=avrtiny -mmcu=atmega8" without any
>> complaints.
>
> IIRC, -march was added because replacing -mmcu=<device> to
> -mmcu=<arch> in the driver's self specs broke multilib selection - the
> driver always acted as if no -mmcu was specified. Adding
> a new option (-march), translating mmcu=<device> to march=<arch> and then
> basing t-multilib on that worked ok.

Okay.  For me it works with -mmcu=, cf. the follow-up patch 
https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00477.html

Two more questions

1)
What is the new macro __AVR_DEV_LIB_NAME__ for?
It uses avr_mcu_t.library_name.  This field contained a part of the crt.o file 
name like "m8" for atmega8's crtm8.o.

The new naming convention is that device libs are located in 
dev/atmega8/libdev.a and startup in dev/atmega8/crt1.o.

The current definition of __AVR_DEV_LIB_NAME__ should be "atmega8" and not 
"m8", hence always that same as __AVR_DEVICE_NAME__.

This macro is not documented in the documentation and useless.  If it is not 
needed and the .library_name field is dead, I'd propose to clean up both.

2)
What about -mpmem-wrap-around resp. ld's --pmem-wrap-around= ?
The wrap around moduli are currently hard coded in specs and just cover a few, 
very old devices.  Presumably that is option-rot?

Does relaxation still need that option to operate correctly?  If not, I'd clean 
that up, too

Johann
diff mbox

Patch

Index: config.gcc
===================================================================
--- config.gcc	(revision 220854)
+++ config.gcc	(working copy)
@@ -1103,7 +1103,6 @@  avr-*-*)
 	fi
 	tmake_file="${tmake_file} avr/t-avr avr/t-multilib"
 	use_gcc_stdint=wrap
-	extra_gcc_objs="driver-avr.o avr-devices.o"
 	extra_objs="avr-devices.o avr-log.o"
 	;;
 bfin*-elf*)
Index: config/avr/t-avr
===================================================================
--- config/avr/t-avr	(revision 221028)
+++ config/avr/t-avr	(working copy)
@@ -16,10 +16,6 @@ 
 # along with GCC; see the file COPYING3.  If not see
 # <http://www.gnu.org/licenses/>.
 
-driver-avr.o: $(srcdir)/config/avr/driver-avr.c \
-  $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H)
-	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(INCLUDES) $<
-
 avr-devices.o: $(srcdir)/config/avr/avr-devices.c \
   $(srcdir)/config/avr/avr-mcus.def \
   $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H)
@@ -69,8 +65,8 @@  gen-avr-mmcu-texi$(build_exeext): $(srcd
 
 gen-avr-mmcu-specs$(build_exeext): $(srcdir)/config/avr/gen-avr-mmcu-specs.c \
   $(AVR_MCUS) $(srcdir)/config/avr/avr-devices.c \
-  $(srcdir)/config/avr/avr-arch.h
-	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $< -o $@
+  $(srcdir)/config/avr/avr-arch.h $(TM_H)
+	$(CXX_FOR_BUILD) $(CXXFLAGS_FOR_BUILD) $< -o $@ $(INCLUDES)
 
 $(srcdir)/doc/avr-mmcu.texi: gen-avr-mmcu-texi$(build_exeext)
 	$(RUN_GEN) ./$< > $@
Index: config/avr/gen-avr-mmcu-specs.c
===================================================================
--- config/avr/gen-avr-mmcu-specs.c	(revision 221028)
+++ config/avr/gen-avr-mmcu-specs.c	(working copy)
@@ -26,9 +26,30 @@ 
 #include "avr-arch.h"
 #include "avr-devices.c"
 
+#define GCC_DEFAULTS_H
+
+#include "tm.h"
+
+#if defined (WITH_AVRLIBC)
+static const bool with_avrlibc = true;
+#else
+static const bool with_avrlibc = false;
+#endif /* WITH_AVRLIBC */
+
+
+/* Return true iff STR starts with PREFIX.  */
+
+static bool
+str_prefix_p (const char *str, const char *prefix)
+{
+  return 0 == strncmp (str, prefix, strlen (prefix));
+}
+
+
 static void
 print_mcu (const avr_mcu_t *mcu)
 {
+  const char *sp8_spec;
   const avr_mcu_t *arch_mcu;
 
   for (arch_mcu = mcu; arch_mcu->macro; )
@@ -37,100 +58,121 @@  for (arch_mcu = mcu; arch_mcu->macro; )
     exit (EXIT_FAILURE);
 
   char name[100];
-  if (snprintf (name, sizeof name, "specs-%s", mcu->name) >= sizeof name)
+  if (snprintf (name, sizeof name, "specs-%s", mcu->name) >= (int) sizeof name)
    exit (EXIT_FAILURE);
 
   FILE *f = fopen (name ,"w");
 
-  const char *sp8, *errata_skip, *rmw;
-  /* Leave "avr2" and "avr25" alone.  These two architectures are
-     the only ones that mix devices with 8-bit SP and 16-bit SP.  */
+  bool errata_skip = 0 != (mcu->dev_attribute & AVR_ERRATA_SKIP);
+  bool rmw = 0 != (mcu->dev_attribute & AVR_ISA_RMW);
+  bool sp8 = 0 != (mcu->dev_attribute & AVR_SHORT_SP);
+
   if (mcu->macro == NULL
       && (mcu->arch == ARCH_AVR2 || mcu->arch == ARCH_AVR25))
-    sp8 = "";
-
-  sp8 = ((mcu->dev_attribute & AVR_SHORT_SP)
-	 ? " -msp8" : " %<msp8");
+    {
+      // Leave "avr2" and "avr25" alone.  These two architectures are
+      // the only ones that mix devices with 8-bit SP and 16-bit SP.
+      sp8_spec = "";
+    }
+  else
+    {
+      sp8_spec = sp8
+        ? " -msp8"
+        : " %<msp8";
+    }
 
-  errata_skip = (mcu->dev_attribute & AVR_ERRATA_SKIP) ? " -mskip-bug" : "";
-  rmw = (mcu->dev_attribute & AVR_ISA_RMW) ? "%{!mno-rmw: -mrmw}" : "";
+  const char *errata_skip_spec = errata_skip
+    ? " %{!mno-skip-bug:-mskip-bug}"
+    : " %{!mskip-bug:-mno-skip-bug}";
+
+  const char *rmw_spec = rmw
+    ? " %{!mno-rmw: -mrmw}"
+    : " %{mrmw}";
 
   const char *arch_name = avr_arch_types[mcu->arch].arch_name;
 
-  fprintf (f, "*self_spec:\n%%{!march=*:-march=%s}%s\n\n", arch_name, sp8);
+  fprintf (f, "*self_spec:\n"
+           " %%{!march=*:-march=%s}"
+           " %s\n\n", arch_name, sp8_spec);
 
   if (mcu->macro)
     fprintf (f, "*cpp:\n-D__AVR_DEV_LIB_NAME__=%s -D%s "
 	     "-D__AVR_DEVICE_NAME__=%s\n\n",
 	     mcu->library_name, mcu->macro, mcu->name);
 
-  fprintf (f, "*cc1:\n%s%s", errata_skip, rmw);
+  fprintf (f, "*cc1:\n%s%s", errata_skip_spec, rmw_spec);
   if (mcu->n_flash != arch_mcu->n_flash)
     fprintf (f, " %%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
   fprintf (f, "\n\n");
-  fprintf (f, "*cc1plus:\n%s%s ", errata_skip, rmw);
+
+  fprintf (f, "*cc1plus:\n%s%s ", errata_skip_spec, rmw_spec);
   if (mcu->n_flash != arch_mcu->n_flash)
-    fprintf (f, "%%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
-  fprintf (f, "%%{!frtti: -fno-rtti}"
-	   "%%{!fenforce-eh-specs: -fno-enforce-eh-specs}"
-	   "%%{!fexceptions: -fno-exceptions}\n\n");
-
-  fprintf (f, "*asm:\n%%{march=*:-mmcu=%%*}%{mrelax: --mlink-relax}%s\n\n",
-	   *errata_skip ? "" : " -mno-skip-bug");
-
-  fprintf (f, "*link:\n%%{mrelax:--relax");
-  if (strncmp (mcu->name, "at90usb8", strlen ("at90usb8")) == 0)
-    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=8k}");
-  if (strncmp (mcu->name, "atmega16", strlen ("atmega16")) == 0)
-    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=16k}");
-  if (strncmp (mcu->name, "atmega32", strlen ("atmega32")) == 0
-      || strncmp (mcu->name, "at90can32", strlen ("at90can32")) == 0)
-    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=32k}");
-  if (strncmp (mcu->name, "atmega64", strlen ("atmega64")) == 0
-      || strncmp (mcu->name, "at90can64", strlen ("at90can64")) == 0
-      || strncmp (mcu->name, "at90usb64", strlen ("at90usb64")) == 0)
-    fprintf (f, "%%{mpmem-wrap-around: --pmem-wrap-around=64k}");
-  fprintf (f, "} %%{march=*:-m%%*}");
+    fprintf (f, " %%{!mn-flash:-mn-flash=%d}", mcu->n_flash);
+  fprintf (f, (" %%{!frtti: -fno-rtti}"
+               " %%{!fenforce-eh-specs: -fno-enforce-eh-specs}"
+               " %%{!fexceptions: -fno-exceptions}\n\n"));
+
+  fprintf (f, "*asm:\n"
+           " %%{march=*:-mmcu=%%*}"
+           " %%{mrelax: --mlink-relax}"
+           " %s%s\n\n", rmw_spec, (errata_skip
+                                  ? " %{mno-skip-bug}"
+                                  : " %{!mskip-bug:-mno-skip-bug}"));
+  fprintf (f, "*link:\n"
+           " %%{mrelax:--relax");
+  {
+    int wrap_k =
+      str_prefix_p (mcu->name, "at90usb8") ? 8
+      : str_prefix_p (mcu->name, "atmega16") ? 16
+      : (str_prefix_p (mcu->name, "atmega32")
+         || str_prefix_p (mcu->name, "at90can32")) ? 32
+      : (str_prefix_p (mcu->name, "atmega64")
+        || str_prefix_p (mcu->name, "at90can64")
+        || str_prefix_p (mcu->name, "at90usb64")) ? 64
+      : 0;
+
+    if (wrap_k)
+      fprintf (f, " %%{mpmem-wrap-around: --pmem-wrap-around=%dk}", wrap_k);
+  }
+  fprintf (f, "}"
+           " %%{march=*:-m%%*}");
+
   if (mcu->data_section_start
       != avr_arch_types[mcu->arch].default_data_section_start)
     fprintf (f, " -Tdata 0x%lX", 0x800000UL + mcu->data_section_start);
+
   if (mcu->text_section_start != 0x0)
-    fprintf (f, " -Ttext 0x%lX", mcu->text_section_start);
+    fprintf (f, " -Ttext 0x%lX", 0UL + mcu->text_section_start);
 
   fprintf (f, " %%{shared:%%eshared is not supported}\n\n");
 
+  bool has_libs = mcu->arch != ARCH_AVR1;
+
   fprintf (f, "*lib:\n");
-  if (strncmp (mcu->name, "mmcu=at90s1", strlen ("mmcu=at90s1")) != 0
-      && strncmp (mcu->name, "mmcu=attiny11", strlen ("mmcu=attiny11")) != 0
-      && strncmp (mcu->name, "mmcu=attiny12", strlen ("mmcu=attiny12")) != 0
-      && strncmp (mcu->name, "mmcu=attiny15", strlen ("mmcu=attiny15")) != 0
-      && strncmp (mcu->name, "mmcu=attiny28", strlen ("mmcu=attiny28")) != 0)
+  if (has_libs)
     {
       fprintf (f, "-lc");
-      if (mcu->macro)
+      if (with_avrlibc
+          && mcu->macro)
 	fprintf (f, " dev/%s/libdev.a%%s", mcu->name);
     }
   fprintf (f, "\n\n");
 
   fprintf (f, "*libgcc:\n");
-  if (strncmp (mcu->name, "mmcu=at90s1", strlen ("mmcu=at90s1")) != 0
-      && strncmp (mcu->name, "mmcu=attiny11", strlen ("mmcu=attiny11")) != 0
-      && strncmp (mcu->name, "mmcu=attiny12", strlen ("mmcu=attiny12")) != 0
-      && strncmp (mcu->name, "mmcu=attiny15", strlen ("mmcu=attiny15")) != 0
-      && strncmp (mcu->name, "mmcu=attiny28", strlen ("mmcu=attiny28")) != 0)
-    fprintf (f, "-lgcc");
+  if (has_libs)
+    fprintf (f, with_avrlibc
+             ? "-lgcc -lm"
+             : "-lgcc");
   fprintf (f, "\n\n");
 
-  fprintf (f, "*startfile:\ndev/%s/crt1.o%%s\n\n", mcu->name);
+  fprintf (f, "*startfile:\n"
+           "dev/%s/crt1.o%%s\n\n", mcu->name);
 }
 
+
 int main (void)
 {
-  enum avr_arch arch = ARCH_UNKNOWN;
-  size_t i, n_mcus = 0;
-  const avr_mcu_t *mcu;
-
-  for (mcu = avr_mcu_types; mcu->name; mcu++)
+  for (const avr_mcu_t *mcu = avr_mcu_types; mcu->name; mcu++)
     print_mcu (mcu);
 
   return EXIT_SUCCESS;
Index: config/avr/driver-avr.c
===================================================================
--- config/avr/driver-avr.c	(revision 221028)
+++ config/avr/driver-avr.c	(working copy)
@@ -1,30 +0,0 @@ 
-/* Subroutines for the gcc driver.
-   Copyright (C) 2009-2015 Free Software Foundation, Inc.
-   Contributed by Anatoly Sokolov <aesok@post.ru>
-
-This file is part of GCC.
-
-GCC is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 3, or (at your option)
-any later version.
-
-GCC is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with GCC; see the file COPYING3.  If not see
-<http://www.gnu.org/licenses/>.  */
-
-#include "config.h"
-#include "system.h"
-#include "coretypes.h"
-#include "tm.h"
-
-/* Current architecture.  */
-const avr_arch_t *avr_current_arch = NULL;
-
-/* Current device.  */
-const avr_mcu_t *avr_current_device = NULL;
Index: config/avr/avrlibc.h
===================================================================
--- config/avr/avrlibc.h	(revision 221028)
+++ config/avr/avrlibc.h	(working copy)
@@ -19,11 +19,13 @@  You should have received a copy of the G
 along with GCC; see the file COPYING3.  If not see
 <http://www.gnu.org/licenses/>.  */
 
-/* AVR-Libc implements functions from libgcc.a in libm.a, see PR54461.  */
+/* AVR-Libc implements functions from libgcc.a in libm.a, see PR54461.
+   More AVR-Libc specific specs originate from gen-avr-mmcu-specs.c:
 
-#undef  LIBGCC_SPEC
-#define LIBGCC_SPEC                                                     \
-  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lgcc -lm }}}}}"
+   - LIBGCC_SPEC (*libgcc)
+   - LIB_SPEC (*lib)
+   
+*/
 
 #undef  LINK_GCC_C_SEQUENCE_SPEC
 #define LINK_GCC_C_SEQUENCE_SPEC \
Index: config/avr/avr.h
===================================================================
--- config/avr/avr.h	(revision 221028)
+++ config/avr/avr.h	(working copy)
@@ -492,42 +492,18 @@  typedef struct avr_args
 #define ADJUST_INSN_LENGTH(INSN, LENGTH)                \
     (LENGTH = avr_adjust_insn_length (INSN, LENGTH))
 
-#define DRIVER_SELF_SPECS " %{mmcu=*:-specs=device-specs/specs-%*%s %<mmcu=*} "
-#define CPP_SPEC ""
+#define DRIVER_SELF_SPECS                                       \
+  " %{!mmcu=*:%{!march=*:-specs=device-specs/specs-avr2%s} "    \
+  "           %{march=*:-specs=device-specs/specs-%*%s}} "      \
+  " %{mmcu=*:-specs=device-specs/specs-%*%s %<mmcu=*} "
 
 /* We want cc1plus used as a preprocessor to pick up the cpp spec from the
    per-device spec files  */
 #define CPLUSPLUS_CPP_SPEC "%(cpp)"
 
-#define CC1_SPEC ""
-
-#define CC1PLUS_SPEC "%{!frtti:-fno-rtti} \
-    %{!fenforce-eh-specs:-fno-enforce-eh-specs} \
-    %{!fexceptions:-fno-exceptions}"
-
-#define ASM_SPEC "%{march=*:-mmcu=%*}%{mrelax: --mlink-relax}"
-  
-#define LINK_SPEC "\
-%{mrelax:--relax\
-         %{mpmem-wrap-around:%{mmcu=at90usb8*:--pmem-wrap-around=8k}\
-                             %{mmcu=atmega16*:--pmem-wrap-around=16k}\
-                             %{mmcu=atmega32*|\
-                               mmcu=at90can32*:--pmem-wrap-around=32k}\
-                             %{mmcu=atmega64*|\
-                               mmcu=at90can64*|\
-                               mmcu=at90usb64*:--pmem-wrap-around=64k}}}\
-%{march=*:-m%*}\
-%{shared:%eshared is not supported}"
-
-#define LIB_SPEC \
-  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lc }}}}}"
-
 #define LIBSTDCXX "gcc"
 /* No libstdc++ for now.  Empty string doesn't work.  */
 
-#define LIBGCC_SPEC \
-  "%{!mmcu=at90s1*:%{!mmcu=attiny11:%{!mmcu=attiny12:%{!mmcu=attiny15:%{!mmcu=attiny28: -lgcc }}}}}"
-
 /* The actual definition will come from the device-specific spec file.  */
 #define STARTFILE_SPEC ""