On 01/30/2015 04:37 PM, Maciej W. Rozycki wrote: > On Fri, 30 Jan 2015, Markos Chandras wrote: > >> When we configure the toolchain, we can set the default >> ISA level to be used when none is set in the command line. >> This, however, has some undesired consequences when the parameters >> used in the command line are incompatible with the built-in ISA >> level of the toolchain. In order to minimize such problems, we set >> a good default ISA level if the Makefile hasn't set one for the >> selected processor. > > Agreed, but does it happen for any actual configuration? If so, then the > configuration is broken and your proposal papers over it, an explicit > `-march=' option is supposed to be there for all the possible CPU_foo > settings. At first look it seems to be the case in arch/mips/Makefile, > but maybe I'm missing something. Besides, a default of `-march=mips32' or > whatever may not really be adequate for the CPU selected. We do have some tools around which default on -march=mips32r6. Then a loongson3_defconfig build gives this kernel/bounds.c:1:0: error: ‘-march=mips32r6’ is not compatible with the selected ABI and that's because in the command line you have no -march=XXXX because there is none set for CPU_LOONGSON3 this is the case I've spotted so far, but if you say that *every* CPU_ symbol needs to set good cflags then this needs to be addressed in a different way I suppose. > >> diff --git a/arch/mips/Makefile b/arch/mips/Makefile >> index 0608ec524d3d..a244fb311a37 100644 >> --- a/arch/mips/Makefile >> +++ b/arch/mips/Makefile >> @@ -226,6 +226,15 @@ cflags-y += -I$(srctree)/arch/mips/include/asm/mach-generic >> drivers-$(CONFIG_PCI) += arch/mips/pci/ >> >> # >> +# Don't trust the toolchain defaults. Use a sensible -march >> +# option but only if we don't have one already. >> +# >> +ifeq (,$(findstring march=, $(cflags-y))) >> +cflags-$(CONFIG_32BIT) += -march=mips32 >> +cflags-$(CONFIG_64BIT) += -march=mips64 >> +endif > > So I'd rather see some form of diagnostics instead, e.g.: > > ifeq (,$(filter -march=% -mips%, $(cflags-y))) > $(error Configuration bug, no `-march=' option set for the CPU selected!) > endif > That looks good to me. I have no strong preference. If that's preferred I will create a new patch -- markos
