[PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

We need to check the ASE support against the correct ISA level
instead of trusting the toolchain will have a good default.

Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 2563a088d3b8..0608ec524d3d 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -131,14 +131,14 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= $(shell $(CC) -dumpmachine |grep -q 'mips.
 # Warning: the 64-bit MIPS architecture does not support the `smartmips' extension
 # Pass -Wa,--no-warn to disable all assembler warnings until the kernel code has
 # been fixed properly.
-cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-msmartmips) -Wa,--no-warn
-cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-mmicromips)
+cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-march=mips32r2 -msmartmips) -Wa,--no-warn
+cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-march=mips32r2 -mmicromips)
 
 cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
 				   -fno-omit-frame-pointer
 
 ifeq ($(CONFIG_CPU_HAS_MSA),y)
-toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64 -Wa$(comma)-mmsa)
+toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float -mfp64 -Wa$(comma)-mmsa)
 cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
 endif
 
-- 
2.2.2

[PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

We need to check the ASE support against the correct ISA level
instead of trusting the toolchain will have a good default.

Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 2563a088d3b8..0608ec524d3d 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -131,14 +131,14 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= $(shell $(CC) -dumpmachine |grep -q 'mips.
 # Warning: the 64-bit MIPS architecture does not support the `smartmips' extension
 # Pass -Wa,--no-warn to disable all assembler warnings until the kernel code has
 # been fixed properly.
-cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-msmartmips) -Wa,--no-warn
-cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-mmicromips)
+cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-march=mips32r2 -msmartmips) -Wa,--no-warn
+cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-march=mips32r2 -mmicromips)
 
 cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
 				   -fno-omit-frame-pointer
 
 ifeq ($(CONFIG_CPU_HAS_MSA),y)
-toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64 -Wa$(comma)-mmsa)
+toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float -mfp64 -Wa$(comma)-mmsa)
 cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
 endif
 
-- 
2.2.2

[PATCH 2/2] MIPS: Makefile: Set default ISA level

[Markos Chandras]

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.

Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)

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
+
+#
 # Automatically detect the build format. By default we choose
 # the elf format according to the load address.
 # We can always force a build with a 64-bits symbol format by
-- 
2.2.2

[PATCH 2/2] MIPS: Makefile: Set default ISA level

[Markos Chandras]

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.

Cc: Matthew Fortune <Matthew.Fortune@imgtec.com>
Signed-off-by: Markos Chandras <markos.chandras@imgtec.com>
---
 arch/mips/Makefile | 9 +++++++++
 1 file changed, 9 insertions(+)

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
+
+#
 # Automatically detect the build format. By default we choose
 # the elf format according to the load address.
 # We can always force a build with a 64-bits symbol format by
-- 
2.2.2

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Markos Chandras wrote:

> @@ -131,14 +131,14 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= $(shell $(CC) -dumpmachine |grep -q 'mips.
>  # Warning: the 64-bit MIPS architecture does not support the `smartmips' extension
>  # Pass -Wa,--no-warn to disable all assembler warnings until the kernel code has
>  # been fixed properly.
> -cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-msmartmips) -Wa,--no-warn
> -cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-mmicromips)
> +cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-march=mips32r2 -msmartmips) -Wa,--no-warn
> +cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-march=mips32r2 -mmicromips)

 The SmartMIPS ASE has been there since r1, e.g. the 4KSd core so you want 
to allow `-march=mips32', but also `-march=mips32r2' if running on earlier 
processors is not needed.

 I think to ensure the right ISA option has been selected it will be the 
best to make it happen in Kconfig, by making CPU_HAS_SMARTMIPS and 
CPU_MICROMIPS depend on the right CPU selection option.  Have you 
considered such an approach (and disregarded it for some reason)?

>  
>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>  				   -fno-omit-frame-pointer
>  
>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64 -Wa$(comma)-mmsa)
> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float -mfp64 -Wa$(comma)-mmsa)
>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
>  endif

 Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?

  Maciej

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

On 01/30/2015 04:20 PM, Maciej W. Rozycki wrote:
> On Fri, 30 Jan 2015, Markos Chandras wrote:
> 
>> @@ -131,14 +131,14 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= $(shell $(CC) -dumpmachine |grep -q 'mips.
>>  # Warning: the 64-bit MIPS architecture does not support the `smartmips' extension
>>  # Pass -Wa,--no-warn to disable all assembler warnings until the kernel code has
>>  # been fixed properly.
>> -cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-msmartmips) -Wa,--no-warn
>> -cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-mmicromips)
>> +cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-march=mips32r2 -msmartmips) -Wa,--no-warn
>> +cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-march=mips32r2 -mmicromips)
> 
>  The SmartMIPS ASE has been there since r1, e.g. the 4KSd core so you want 
> to allow `-march=mips32', but also `-march=mips32r2' if running on earlier 
> processors is not needed.
> 
>  I think to ensure the right ISA option has been selected it will be the 
> best to make it happen in Kconfig, by making CPU_HAS_SMARTMIPS and 
> CPU_MICROMIPS depend on the right CPU selection option.  Have you 
> considered such an approach (and disregarded it for some reason)?

I considered it but i thought passing something sane to $(call
cc-option) might be preferred. What I am trying to do here is to ensure
the $(call cc-option) will not fail in case your toolchain really
supports micromips or smartmips but when combined with a bad default it
simply fails

> 
>>  
>>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>>  				   -fno-omit-frame-pointer
>>  
>>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
>> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64 -Wa$(comma)-mmsa)
>> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float -mfp64 -Wa$(comma)-mmsa)
>>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
>>  endif
> 
>  Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?
I am not sure but like I explained above, it does not have to be 100%
accurate. Just something to keep your toolchain happy and really enable
MSA support even if you happen and old ISA level as the default one for
your toolchain.

for example, if your toolchain has -march=mips2 as default then

-mhard-float -mfp64 will fail

but

-march=mips32r2 -mhard-float -mfp64

will pass. Your toolchain does support MSA, but because you combined the
check with incompatible flags, then the end result is not what you want.

I am open to suggestions if you want to solve this in a better way.

-- 
markos

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

On 01/30/2015 04:20 PM, Maciej W. Rozycki wrote:
> On Fri, 30 Jan 2015, Markos Chandras wrote:
> 
>> @@ -131,14 +131,14 @@ cflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= $(shell $(CC) -dumpmachine |grep -q 'mips.
>>  # Warning: the 64-bit MIPS architecture does not support the `smartmips' extension
>>  # Pass -Wa,--no-warn to disable all assembler warnings until the kernel code has
>>  # been fixed properly.
>> -cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-msmartmips) -Wa,--no-warn
>> -cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-mmicromips)
>> +cflags-$(CONFIG_CPU_HAS_SMARTMIPS)	+= $(call cc-option,-march=mips32r2 -msmartmips) -Wa,--no-warn
>> +cflags-$(CONFIG_CPU_MICROMIPS) += $(call cc-option,-march=mips32r2 -mmicromips)
> 
>  The SmartMIPS ASE has been there since r1, e.g. the 4KSd core so you want 
> to allow `-march=mips32', but also `-march=mips32r2' if running on earlier 
> processors is not needed.
> 
>  I think to ensure the right ISA option has been selected it will be the 
> best to make it happen in Kconfig, by making CPU_HAS_SMARTMIPS and 
> CPU_MICROMIPS depend on the right CPU selection option.  Have you 
> considered such an approach (and disregarded it for some reason)?

I considered it but i thought passing something sane to $(call
cc-option) might be preferred. What I am trying to do here is to ensure
the $(call cc-option) will not fail in case your toolchain really
supports micromips or smartmips but when combined with a bad default it
simply fails

> 
>>  
>>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>>  				   -fno-omit-frame-pointer
>>  
>>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
>> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64 -Wa$(comma)-mmsa)
>> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float -mfp64 -Wa$(comma)-mmsa)
>>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
>>  endif
> 
>  Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?
I am not sure but like I explained above, it does not have to be 100%
accurate. Just something to keep your toolchain happy and really enable
MSA support even if you happen and old ISA level as the default one for
your toolchain.

for example, if your toolchain has -march=mips2 as default then

-mhard-float -mfp64 will fail

but

-march=mips32r2 -mhard-float -mfp64

will pass. Your toolchain does support MSA, but because you combined the
check with incompatible flags, then the end result is not what you want.

I am open to suggestions if you want to solve this in a better way.

-- 
markos

Re: [PATCH 2/2] MIPS: Makefile: Set default ISA level

[Maciej W. Rozycki]

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.

> 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

or suchlike (`-mips%' for the legacy stuff; we should probably drop it 
sometime).  WDYT?

  Maciej

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Markos Chandras wrote:

> >>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
> >>  				   -fno-omit-frame-pointer
> >>  
> >>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
> >> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64
> -Wa$(comma)-mmsa)
> >> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float
> -mfp64 -Wa$(comma)-mmsa)
> >>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
> >>  endif
> > 
> >  Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?
> I am not sure but like I explained above, it does not have to be 100%
> accurate. Just something to keep your toolchain happy and really enable
> MSA support even if you happen and old ISA level as the default one for
> your toolchain.
> 
> for example, if your toolchain has -march=mips2 as default then
> 
> -mhard-float -mfp64 will fail
> 
> but
> 
> -march=mips32r2 -mhard-float -mfp64
> 
> will pass. Your toolchain does support MSA, but because you combined the
> check with incompatible flags, then the end result is not what you want.

 Hmm doesn't `cc-option-yn' pull options accumulated in $(cflags-y) 
already for the check it makes?  If not, then it's rather less than useful 
for us and I can see two options here:

1. Find or make a version of the function that does.

2. Find or make a version of the function that takes extra options used 
   for the duration of the check only and not propagated to its output.

I think the latter option might be a bit better as we can choose a set of 
options that guarantees success with a sufficiently modern toolchain so 
that the option intended is included regardless of any configuration fault 
elsewhere, that will then likely manifest itself loudly rather than 
lingering unnoticed and possibly only causing issues at the run time.

  Maciej

Re: [PATCH 2/2] MIPS: Makefile: Set default ISA level

[Markos Chandras]

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

Re: [PATCH 2/2] MIPS: Makefile: Set default ISA level

[Markos Chandras]

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

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

On 01/30/2015 04:50 PM, Maciej W. Rozycki wrote:
> On Fri, 30 Jan 2015, Markos Chandras wrote:
> 
>>>>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>>>>  				   -fno-omit-frame-pointer
>>>>  
>>>>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
>>>> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64
>> -Wa$(comma)-mmsa)
>>>> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float
>> -mfp64 -Wa$(comma)-mmsa)
>>>>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
>>>>  endif
>>>
>>>  Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?
>> I am not sure but like I explained above, it does not have to be 100%
>> accurate. Just something to keep your toolchain happy and really enable
>> MSA support even if you happen and old ISA level as the default one for
>> your toolchain.
>>
>> for example, if your toolchain has -march=mips2 as default then
>>
>> -mhard-float -mfp64 will fail
>>
>> but
>>
>> -march=mips32r2 -mhard-float -mfp64
>>
>> will pass. Your toolchain does support MSA, but because you combined the
>> check with incompatible flags, then the end result is not what you want.
> 
>  Hmm doesn't `cc-option-yn' pull options accumulated in $(cflags-y) 
> already for the check it makes?  If not, then it's rather less than useful 
> for us and I can see two options here:

I think it does but IIRC at this point no -march=XXXX was set. Perhaps
moving the ASEs checks after the CPU_* code in Makefile to fix this
problem indirectly.

-- 
markos

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Markos Chandras]

On 01/30/2015 04:50 PM, Maciej W. Rozycki wrote:
> On Fri, 30 Jan 2015, Markos Chandras wrote:
> 
>>>>  cflags-$(CONFIG_SB1XXX_CORELIS)	+= $(call cc-option,-mno-sched-prolog) \
>>>>  				   -fno-omit-frame-pointer
>>>>  
>>>>  ifeq ($(CONFIG_CPU_HAS_MSA),y)
>>>> -toolchain-msa	:= $(call cc-option-yn,-mhard-float -mfp64
>> -Wa$(comma)-mmsa)
>>>> +toolchain-msa	:= $(call cc-option-yn,-march=mips32r2 -mhard-float
>> -mfp64 -Wa$(comma)-mmsa)
>>>>  cflags-$(toolchain-msa)		+= -DTOOLCHAIN_SUPPORTS_MSA
>>>>  endif
>>>
>>>  Similarly here, is CPU_HAS_MSA incompatible with `-march=mips64r2'?
>> I am not sure but like I explained above, it does not have to be 100%
>> accurate. Just something to keep your toolchain happy and really enable
>> MSA support even if you happen and old ISA level as the default one for
>> your toolchain.
>>
>> for example, if your toolchain has -march=mips2 as default then
>>
>> -mhard-float -mfp64 will fail
>>
>> but
>>
>> -march=mips32r2 -mhard-float -mfp64
>>
>> will pass. Your toolchain does support MSA, but because you combined the
>> check with incompatible flags, then the end result is not what you want.
> 
>  Hmm doesn't `cc-option-yn' pull options accumulated in $(cflags-y) 
> already for the check it makes?  If not, then it's rather less than useful 
> for us and I can see two options here:

I think it does but IIRC at this point no -march=XXXX was set. Perhaps
moving the ASEs checks after the CPU_* code in Makefile to fix this
problem indirectly.

-- 
markos

Re: [PATCH 2/2] MIPS: Makefile: Set default ISA level

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Markos Chandras wrote:

> >  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.

 In that case I'd expect `-march=loongson3a' or whatever is appropriate 
for the architecture to be set.  If a fallback is required for older 
toolchains, then an arrangement similar to one for CPU_SB1 can be made.
E.g.:

cflags-$(CONFIG_CPU_LOONGSON3)	+= $(call cc-option,-march=loongson3a,-march=mips64r2) -Wa,--trap

or suchlike (based on what GCC says about it).

  Maciej

Re: [PATCH 1/2] MIPS: Makefile: Set correct ISA level for MIPS ASEs

[Maciej W. Rozycki]

On Fri, 30 Jan 2015, Markos Chandras wrote:

> >  Hmm doesn't `cc-option-yn' pull options accumulated in $(cflags-y) 
> > already for the check it makes?  If not, then it's rather less than useful 
> > for us and I can see two options here:
> 
> I think it does but IIRC at this point no -march=XXXX was set. Perhaps
> moving the ASEs checks after the CPU_* code in Makefile to fix this
> problem indirectly.

 That sounds like a pragmatic fix to me then!

  Maciej