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
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 <[email protected]>
---
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,)
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 <[email protected]>
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
Cc: [email protected] # 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)
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 <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Signed-off-by: Maciej W. Rozycki <[email protected]>
Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
Cc: [email protected] # 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')
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 <[email protected]> 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
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 <[email protected]>
> Signed-off-by: Maciej W. Rozycki <[email protected]>
> Fixes: 805b2e1d427a ("kbuild: include Makefile.compiler only when compiler is needed")
> Cc: [email protected] # v5.13+
> ---
> arch/mips/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
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
Hi, Maciej,
On Wed, Jul 19, 2023 at 11:39 PM Maciej W. Rozycki <[email protected]> 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
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
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 ]