[PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Maciej W. Rozycki]

Hi,

 With the addition of the `need-compiler' variable the `Makefile.compiler' 
fragment is not included with no-build targets such as `modules_install', 
which in turn means $(call cc-option,), etc. are no-ops with these targets 
and any attempt to evaluate these function calls causes all kinds of weird 
behaviour to happen.

 The solution is to avoid making these calls in the first place, as they 
are surely irrelevant where the compiler is not going to be otherwise 
invoked.  This small patch series fixes two places known-affected in the 
MIPS Makefile fragment and also included a follow-up revert of an earlier 
misguided attempt.  See individual change descriptions for details.

 Verified with `decstation_64_defconfig' and `fuloong2e_defconfig' using 
`modules_install'.  Please apply.

  Maciej

[PATCH 1/3] MIPS: Fix CONFIG_CPU_DADDI_WORKAROUNDS `modules_install' regression

[Maciej W. Rozycki]

Remove a build-time check for the presence of the GCC `-msym32' option.  
This option has been there since GCC 4.1.0, which is below the minimum 
required as at commit 805b2e1d427a ("kbuild: include Makefile.compiler 
only when compiler is needed"), when an error message:

arch/mips/Makefile:306: *** CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32.  Stop.

started to trigger for the `modules_install' target with configurations 
such as `decstation_64_defconfig' that set CONFIG_CPU_DADDI_WORKAROUNDS, 
because said commit has made `cc-option-yn' an undefined function for 
non-build targets.

Reported-by: Jan-Benedict Glaw <jbglaw@lug-owl.de>
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
Cc: stable@vger.kernel.org # v5.13+
---
 arch/mips/Makefile |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

linux-mips-msym32-uncond.diff
Index: linux-macro/arch/mips/Makefile
===================================================================
--- linux-macro.orig/arch/mips/Makefile
+++ linux-macro/arch/mips/Makefile
@@ -299,8 +299,8 @@ ifdef CONFIG_64BIT
     endif
   endif
 
-  ifeq ($(KBUILD_SYM32)$(call cc-option-yn,-msym32), yy)
-    cflags-y += -msym32 -DKBUILD_64BIT_SYM32
+  ifeq ($(KBUILD_SYM32), y)
+    cflags-$(KBUILD_SYM32) += -msym32 -DKBUILD_64BIT_SYM32
   else
     ifeq ($(CONFIG_CPU_DADDI_WORKAROUNDS), y)
       $(error CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32)

Re: [PATCH 1/3] MIPS: Fix CONFIG_CPU_DADDI_WORKAROUNDS `modules_install' regression

[Philippe Mathieu-Daudé]

On 18/7/23 16:37, Maciej W. Rozycki wrote:
> Remove a build-time check for the presence of the GCC `-msym32' option.
> This option has been there since GCC 4.1.0, which is below the minimum
> required as at commit 805b2e1d427a ("kbuild: include Makefile.compiler
> only when compiler is needed"), when an error message:
> 
> arch/mips/Makefile:306: *** CONFIG_CPU_DADDI_WORKAROUNDS unsupported without -msym32.  Stop.
> 
> started to trigger for the `modules_install' target with configurations
> such as `decstation_64_defconfig' that set CONFIG_CPU_DADDI_WORKAROUNDS,
> because said commit has made `cc-option-yn' an undefined function for
> non-build targets.
> 
> Reported-by: Jan-Benedict Glaw <jbglaw@lug-owl.de>
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
> Cc: stable@vger.kernel.org # v5.13+
> ---
>   arch/mips/Makefile |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 2/3] MIPS: Only fiddle with CHECKFLAGS if `need-compiler'

[Maciej W. Rozycki]

We have originally guarded fiddling with CHECKFLAGS in our arch Makefile 
by checking for the CONFIG_MIPS variable, not set for targets such as 
`distclean', etc. that neither include `.config' nor use the compiler.  

Starting from commit 805b2e1d427a ("kbuild: include Makefile.compiler 
only when compiler is needed") we have had a generic `need-compiler' 
variable explicitly telling us if the compiler will be used and thus its 
capabilities need to be checked and expressed in the form of compilation
flags.  If this variable is not set, then `make' functions such as 
`cc-option' are undefined, causing all kinds of weirdness to happen if 
we expect specific results to be returned, most recently:

cc1: error: '-mloongson-mmi' must be used with '-mhard-float'

messages with configurations such as `fuloong2e_defconfig' and the 
`modules_install' target, which does include `.config' and yet does not 
use the compiler.

Replace the check for CONFIG_MIPS with one for `need-compiler' instead, 
so as to prevent the compiler from being ever called for CHECKFLAGS when 
not needed.

Reported-by: Guillaume Tucker <guillaume.tucker@collabora.com>
Closes: https://lore.kernel.org/r/85031c0c-d981-031e-8a50-bc4fad2ddcd8@collabora.com/
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
Cc: stable@vger.kernel.org # v5.13+
---
 arch/mips/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

linux-mips-checkflags-need-compiler.diff
Index: linux-macro/arch/mips/Makefile
===================================================================
--- linux-macro.orig/arch/mips/Makefile
+++ linux-macro/arch/mips/Makefile
@@ -341,7 +341,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwin
 
 KBUILD_LDFLAGS		+= -m $(ld-emul)
 
-ifdef CONFIG_MIPS
+ifdef need-compiler
 CHECKFLAGS += $(shell $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
 	grep -E -vw '__GNUC_(MINOR_|PATCHLEVEL_)?_' | \
 	sed -e "s/^\#define /-D'/" -e "s/ /'='/" -e "s/$$/'/" -e 's/\$$/&&/g')

Re: [PATCH 2/3] MIPS: Only fiddle with CHECKFLAGS if `need-compiler'

[Guillaume Tucker]

Please also mention the KernelCI bot as this was initially found
thanks to an automated email report:

On 18/07/2023 16:37, Maciej W. Rozycki wrote:
> Reported-by: Guillaume Tucker <guillaume.tucker@collabora.com>

Reported-by: "kernelci.org bot" <bot@kernelci.org>

Thanks,
Guillaume

[PATCH 3/3] Revert MIPS: Loongson: Fix build error when make modules_install

[Maciej W. Rozycki]

Revert commit 531b3d1195d0 ("MIPS: Loongson: Fix build error when make 
modules_install"), which made `-march=loongson2e', `-march=loongson2f', 
and `-march=loongson3a' compilation options probed for even though GCC 
has supported them since 4.4.0, 4.4.0, and 4.6.0 respectively, which is 
below our current minimum requirement of 5.1, in an attempt to work 
around for the `cc-option' `make' function being undefined with `make' 
targets that do not use the compiler.  The workaround has now been made 
obsolete, by querying the `need-compiler' variable instead so as to make 
sure the compiler isn't called for non-build targets.

Verified with `fuloong2e_defconfig' and the `modules_install' target.

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
---
 arch/mips/Makefile |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: linux-macro/arch/mips/Makefile
===================================================================
--- linux-macro.orig/arch/mips/Makefile
+++ linux-macro/arch/mips/Makefile
@@ -181,12 +181,16 @@ endif
 cflags-$(CONFIG_CAVIUM_CN63XXP1) += -Wa,-mfix-cn63xxp1
 cflags-$(CONFIG_CPU_BMIPS)	+= -march=mips32 -Wa,-mips32 -Wa,--trap
 
-cflags-$(CONFIG_CPU_LOONGSON2E) += $(call cc-option,-march=loongson2e) -Wa,--trap
-cflags-$(CONFIG_CPU_LOONGSON2F) += $(call cc-option,-march=loongson2f) -Wa,--trap
-cflags-$(CONFIG_CPU_LOONGSON64) += $(call cc-option,-march=loongson3a,-march=mips64r2) -Wa,--trap
+cflags-$(CONFIG_CPU_LOONGSON2E) += -march=loongson2e -Wa,--trap
+cflags-$(CONFIG_CPU_LOONGSON2F) += -march=loongson2f -Wa,--trap
 # Some -march= flags enable MMI instructions, and GCC complains about that
 # support being enabled alongside -msoft-float. Thus explicitly disable MMI.
 cflags-$(CONFIG_CPU_LOONGSON2EF) += $(call cc-option,-mno-loongson-mmi)
+ifdef CONFIG_CPU_LOONGSON64
+cflags-$(CONFIG_CPU_LOONGSON64)	+= -Wa,--trap
+cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
+cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2
+endif
 cflags-$(CONFIG_CPU_LOONGSON64) += $(call cc-option,-mno-loongson-mmi)
 
 cflags-$(CONFIG_CPU_R4000_WORKAROUNDS)	+= $(call cc-option,-mfix-r4000,)

Re: [PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Huacai Chen]

Hi, Maciej,

Even if patch-2 resolves the problem, I don't think patch-3 is
necessary because the original patch makes code simpler and more
compact.

Huacai

On Tue, Jul 18, 2023 at 10:42 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> Hi,
>
>  With the addition of the `need-compiler' variable the `Makefile.compiler'
> fragment is not included with no-build targets such as `modules_install',
> which in turn means $(call cc-option,), etc. are no-ops with these targets
> and any attempt to evaluate these function calls causes all kinds of weird
> behaviour to happen.
>
>  The solution is to avoid making these calls in the first place, as they
> are surely irrelevant where the compiler is not going to be otherwise
> invoked.  This small patch series fixes two places known-affected in the
> MIPS Makefile fragment and also included a follow-up revert of an earlier
> misguided attempt.  See individual change descriptions for details.
>
>  Verified with `decstation_64_defconfig' and `fuloong2e_defconfig' using
> `modules_install'.  Please apply.
>
>   Maciej

Re: [PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Maciej W. Rozycki]

On Tue, 18 Jul 2023, Huacai Chen wrote:

> Even if patch-2 resolves the problem, I don't think patch-3 is
> necessary because the original patch makes code simpler and more
> compact.

 Please don't top-post on a public mailing list.

 If you're referring to this part:

+ifdef CONFIG_CPU_LOONGSON64
+cflags-$(CONFIG_CPU_LOONGSON64)	+= -Wa,--trap
+cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
+cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2
+endif

then it can be done with a separate clean-up.  Otherwise it'll have been 
lost in the noise.

 Firstly:

cflags-$(CONFIG_CPU_LOONGSON64)	+= -Wa,--trap

doesn't have to be wrapped in `ifdef CONFIG_CPU_LOONGSON64'.

 Secondly:

cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2

document compiler peculiarities.  Does Clang support, or intend to, 
`-march=loongson3a'?  If so, what version can we expect this stuff in?  
GCC has had it since 4.6 or Y2010, which is pretty long ago.

 Last but not least there are formatting anomalies there, which may have 
to be dealt with in a separate change.

  Maciej

Re: [PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Huacai Chen]

Hi, Maciej,

On Wed, Jul 19, 2023 at 11:39 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Tue, 18 Jul 2023, Huacai Chen wrote:
>
> > Even if patch-2 resolves the problem, I don't think patch-3 is
> > necessary because the original patch makes code simpler and more
> > compact.
>
>  Please don't top-post on a public mailing list.
>
>  If you're referring to this part:
>
> +ifdef CONFIG_CPU_LOONGSON64
> +cflags-$(CONFIG_CPU_LOONGSON64)        += -Wa,--trap
> +cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
> +cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2
> +endif
>
> then it can be done with a separate clean-up.  Otherwise it'll have been
> lost in the noise.
>
>  Firstly:
>
> cflags-$(CONFIG_CPU_LOONGSON64) += -Wa,--trap
>
> doesn't have to be wrapped in `ifdef CONFIG_CPU_LOONGSON64'.
>
>  Secondly:
>
> cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
> cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2
>
> document compiler peculiarities.  Does Clang support, or intend to,
> `-march=loongson3a'?  If so, what version can we expect this stuff in?
> GCC has had it since 4.6 or Y2010, which is pretty long ago.
GCC support loongson3a/mips64r2, Clang only support mips64r2. If we use
$(call cc-option,-march=loongson3a,-march=mips64r2)
both GCC and Clang can work and we don't need to care about the compiler.

Huacai

>
>  Last but not least there are formatting anomalies there, which may have
> to be dealt with in a separate change.
>
>   Maciej

Re: [PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Maciej W. Rozycki]

Hi Huacai,

> >  Secondly:
> >
> > cflags-$(CONFIG_CC_IS_GCC) += -march=loongson3a
> > cflags-$(CONFIG_CC_IS_CLANG) += -march=mips64r2
> >
> > document compiler peculiarities.  Does Clang support, or intend to,
> > `-march=loongson3a'?  If so, what version can we expect this stuff in?
> > GCC has had it since 4.6 or Y2010, which is pretty long ago.
> GCC support loongson3a/mips64r2, Clang only support mips64r2. If we use
> $(call cc-option,-march=loongson3a,-march=mips64r2)
> both GCC and Clang can work and we don't need to care about the compiler.

 This may well be a change we desire, but it has to be made and reviewed 
on its own rather than being buried within a set of unrelated changes.  
Then the rationale has to be given in the change description and a comment 
put in code explaining that it's not the usual case of old/new compiler, 
so that we can catch it later and remove should Clang developers decide to 
include `-march=loongson3a' support and our version requirements catch up.

  Maciej

Re: [PATCH 0/3] MIPS: Fix build issues from the introduction of `need-compiler'

[Thomas Bogendoerfer]

On Tue, Jul 18, 2023 at 03:37:13PM +0100, Maciej W. Rozycki wrote:
> Hi,
> 
>  With the addition of the `need-compiler' variable the `Makefile.compiler' 
> fragment is not included with no-build targets such as `modules_install', 
> which in turn means $(call cc-option,), etc. are no-ops with these targets 
> and any attempt to evaluate these function calls causes all kinds of weird 
> behaviour to happen.
> 
>  The solution is to avoid making these calls in the first place, as they 
> are surely irrelevant where the compiler is not going to be otherwise 
> invoked.  This small patch series fixes two places known-affected in the 
> MIPS Makefile fragment and also included a follow-up revert of an earlier 
> misguided attempt.  See individual change descriptions for details.
> 
>  Verified with `decstation_64_defconfig' and `fuloong2e_defconfig' using 
> `modules_install'.  Please apply.

series applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]