2023-08-05 19:29:46

by Zhangjin Wu

[permalink] [raw]
Subject: [PATCH v6 7/8] selftests/nolibc: allow customize CROSS_COMPILE by architecture

Some cross compilers may not just be prefixed with ARCH, customize them
by architecture may simplify the test a lot, especially, when iterate
with ARCH.

After customizing this for every architecture, the minimal test argument
will be architecture itself, no CROSS_COMPILE required to be passed.

If the prefix of installed cross compiler is not the same as the one
customized, we can also pass CROSS_COMPILE as before or even pass
CROSS_COMPILE_<ARCH>.

Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 452d8e424d28..45d231b9c5c2 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -55,6 +55,12 @@ IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
IMAGE = $(IMAGE_$(XARCH))
IMAGE_NAME = $(notdir $(IMAGE))

+# CROSS_COMPILE: cross toolchain prefix by architecture
+CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH))
+
+# make sure CC is prefixed with CROSS_COMPILE
+$(call allow-override,CC,$(CROSS_COMPILE)gcc)
+
# default kernel configurations that appear to be usable
DEFCONFIG_i386 = defconfig
DEFCONFIG_x86_64 = defconfig
--
2.25.1



2023-08-06 08:38:23

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v6 7/8] selftests/nolibc: allow customize CROSS_COMPILE by architecture

On Sun, Aug 06, 2023 at 02:46:03AM +0800, Zhangjin Wu wrote:
> Some cross compilers may not just be prefixed with ARCH, customize them
> by architecture may simplify the test a lot, especially, when iterate
> with ARCH.
>
> After customizing this for every architecture, the minimal test argument
> will be architecture itself, no CROSS_COMPILE required to be passed.
>
> If the prefix of installed cross compiler is not the same as the one
> customized, we can also pass CROSS_COMPILE as before or even pass
> CROSS_COMPILE_<ARCH>.
>
> Signed-off-by: Zhangjin Wu <[email protected]>
> ---
> tools/testing/selftests/nolibc/Makefile | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 452d8e424d28..45d231b9c5c2 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -55,6 +55,12 @@ IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
> IMAGE = $(IMAGE_$(XARCH))
> IMAGE_NAME = $(notdir $(IMAGE))
>
> +# CROSS_COMPILE: cross toolchain prefix by architecture
> +CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH))
> +
> +# make sure CC is prefixed with CROSS_COMPILE
> +$(call allow-override,CC,$(CROSS_COMPILE)gcc)
> +

Note that I feared that it would break my "CC=gcc-5.5.0" and so on but
fortunately it did not. However I don't understand what it tries to do
nor the problem it tries to solve (even the commit message is quite
unclear to me). I'm leaving it aside anyway but I wanted to let you
know.

Willy

2023-08-06 10:00:37

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v6 7/8] selftests/nolibc: allow customize CROSS_COMPILE by architecture

Hi, Willy

> On Sun, Aug 06, 2023 at 02:46:03AM +0800, Zhangjin Wu wrote:
> > Some cross compilers may not just be prefixed with ARCH, customize them
> > by architecture may simplify the test a lot, especially, when iterate
> > with ARCH.
> >
> > After customizing this for every architecture, the minimal test argument
> > will be architecture itself, no CROSS_COMPILE required to be passed.
> >
> > If the prefix of installed cross compiler is not the same as the one
> > customized, we can also pass CROSS_COMPILE as before or even pass
> > CROSS_COMPILE_<ARCH>.
> >
> > Signed-off-by: Zhangjin Wu <[email protected]>
> > ---
> > tools/testing/selftests/nolibc/Makefile | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 452d8e424d28..45d231b9c5c2 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -55,6 +55,12 @@ IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
> > IMAGE = $(IMAGE_$(XARCH))
> > IMAGE_NAME = $(notdir $(IMAGE))
> >
> > +# CROSS_COMPILE: cross toolchain prefix by architecture
> > +CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH))
> > +
> > +# make sure CC is prefixed with CROSS_COMPILE
> > +$(call allow-override,CC,$(CROSS_COMPILE)gcc)
> > +
>
> Note that I feared that it would break my "CC=gcc-5.5.0" and so on but
> fortunately it did not. However I don't understand what it tries to do
> nor the problem it tries to solve (even the commit message is quite
> unclear to me). I'm leaving it aside anyway but I wanted to let you
> know.
>

Thanks you for this note, will add it as a test case in our v2.

These places require the '$(CC)' prefixed with $(CROSS_COMPILE):

$ grep ")\$(CC)" tools/testing/selftests/nolibc/Makefile
$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
$(QUIET_CC)$(CC) -o $@ $<

When CROSS_COMPILE come from command line, the first lines we included have
the CROSS_COMPILE knowledge and will add right prefix for CC:

# Makefile for nolibc tests
include ../../../scripts/Makefile.include

But the customized $(CROSS_COMPILE) must be added from the XARCH lines, then,
differs from the ones passed from command line, the above lines (before XARCH)
will have no CROSS_COMPILE knowledge, the CC therefore will have no prefix.

CROSS_COMPILE=xxx <= before, from command line

include ../../../scripts/Makefile.include <= CC get the right CROSS_COMPILE prefix

XARCH = $(or $(XARCH_$(ARCH)),$(ARCH)) <= XARCH here

CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH)) <= after, customize ourselves
(call allow-override,CC,$(CROSS_COMPILE)gcc) <= CC have no right prefix here

$(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \

So, we must add the prefix to CC ourselves after the CROSS_COMPILE lines we
customized, the 'allow-override' helper is also from
../../../scripts/Makefile.include.

But you did find a bug above, we should include it again here to avoid not
break the possibility of using llvm (still require to check if there are some
other regressions):

include ../../../scripts/Makefile.include

And I have further found there is another cc-cross-prefix helper from:

$ grep cc-cross-prefix -ur scripts/
scripts/Makefile.compiler:# cc-cross-prefix
scripts/Makefile.compiler:# Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-)
scripts/Makefile.compiler:cc-cross-prefix = $(firstword $(foreach c, $(1), \

So, we are able to search the toolchains from Arnd's, local toolchains and ...,
may not need to force users to use which one, I will do more tests on it.

Please don't merge this patch too, to avoid break anything, let's tune it
carefully in our v2 and delay the whole stuff to v6.7.

Thanks,
Zhangjin

> Willy

2023-08-06 11:08:25

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v6 7/8] selftests/nolibc: allow customize CROSS_COMPILE by architecture

On Sun, Aug 06, 2023 at 05:39:21PM +0800, Zhangjin Wu wrote:
> > > +# CROSS_COMPILE: cross toolchain prefix by architecture
> > > +CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH))
> > > +
> > > +# make sure CC is prefixed with CROSS_COMPILE
> > > +$(call allow-override,CC,$(CROSS_COMPILE)gcc)
> > > +
> >
> > Note that I feared that it would break my "CC=gcc-5.5.0" and so on but
> > fortunately it did not. However I don't understand what it tries to do
> > nor the problem it tries to solve (even the commit message is quite
> > unclear to me). I'm leaving it aside anyway but I wanted to let you
> > know.
> >
>
> Thanks you for this note, will add it as a test case in our v2.
>
> These places require the '$(CC)' prefixed with $(CROSS_COMPILE):
>
> $ grep ")\$(CC)" tools/testing/selftests/nolibc/Makefile
> $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
> $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
> $(QUIET_CC)$(CC) -o $@ $<
>
> When CROSS_COMPILE come from command line, the first lines we included have
> the CROSS_COMPILE knowledge and will add right prefix for CC:
>
> # Makefile for nolibc tests
> include ../../../scripts/Makefile.include
>
> But the customized $(CROSS_COMPILE) must be added from the XARCH lines, then,
> differs from the ones passed from command line, the above lines (before XARCH)
> will have no CROSS_COMPILE knowledge, the CC therefore will have no prefix.
>
> CROSS_COMPILE=xxx <= before, from command line
>
> include ../../../scripts/Makefile.include <= CC get the right CROSS_COMPILE prefix
>
> XARCH = $(or $(XARCH_$(ARCH)),$(ARCH)) <= XARCH here
>
> CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH)) <= after, customize ourselves
> (call allow-override,CC,$(CROSS_COMPILE)gcc) <= CC have no right prefix here
>
> $(QUIET_CC)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ \
>
> So, we must add the prefix to CC ourselves after the CROSS_COMPILE lines we
> customized, the 'allow-override' helper is also from
> ../../../scripts/Makefile.include.

OK, I feared it would overwrite the command-line one.

> But you did find a bug above, we should include it again here to avoid not
> break the possibility of using llvm (still require to check if there are some
> other regressions):
>
> include ../../../scripts/Makefile.include
>
> And I have further found there is another cc-cross-prefix helper from:
>
> $ grep cc-cross-prefix -ur scripts/
> scripts/Makefile.compiler:# cc-cross-prefix
> scripts/Makefile.compiler:# Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-)
> scripts/Makefile.compiler:cc-cross-prefix = $(firstword $(foreach c, $(1), \
>
> So, we are able to search the toolchains from Arnd's, local toolchains and ...,
> may not need to force users to use which one, I will do more tests on it.
>
> Please don't merge this patch too, to avoid break anything, let's tune it
> carefully in our v2 and delay the whole stuff to v6.7.

OK. Note that in the end it might be less difficult to try to set
CROSS_COMPILE *before* including the general include instead of
after: we could preset CROSS_COMPILE based on the ARCH/XARCH we know
at this step, as this is not expected to rely on auto-detection.

Willy

2023-08-06 11:09:53

by Zhangjin Wu

[permalink] [raw]
Subject: Re: [PATCH v6 7/8] selftests/nolibc: allow customize CROSS_COMPILE by architecture

It uses 'eval $(1) = $(2)' style, no real 'override' as the name shows
;-)

> > But you did find a bug above, we should include it again here to avoid not
> > break the possibility of using llvm (still require to check if there are some
> > other regressions):
> >
> > include ../../../scripts/Makefile.include
> >
> > And I have further found there is another cc-cross-prefix helper from:
> >
> > $ grep cc-cross-prefix -ur scripts/
> > scripts/Makefile.compiler:# cc-cross-prefix
> > scripts/Makefile.compiler:# Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-)
> > scripts/Makefile.compiler:cc-cross-prefix = $(firstword $(foreach c, $(1), \
> >
> > So, we are able to search the toolchains from Arnd's, local toolchains and ...,
> > may not need to force users to use which one, I will do more tests on it.
> >
> > Please don't merge this patch too, to avoid break anything, let's tune it
> > carefully in our v2 and delay the whole stuff to v6.7.
>
> OK. Note that in the end it might be less difficult to try to set
> CROSS_COMPILE *before* including the general include instead of
> after: we could preset CROSS_COMPILE based on the ARCH/XARCH we know
> at this step, as this is not expected to rely on auto-detection.
>

A simple tests shows, we are able to simply move the include after our
customize lines:

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 1f7c36fbe083..ed21dc393dc0 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -1,6 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-# Makefile for nolibc tests
-include ../../../scripts/Makefile.include
# We need this for the "cc-option" macro.
include ../../../build/Build.include

diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 228a95e65113..91a2a546954c 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -55,6 +55,12 @@ IMAGE_loongarch = arch/loongarch/boot/vmlinuz.efi
IMAGE = $(IMAGE_$(XARCH))
IMAGE_NAME = $(notdir $(IMAGE))

+# CROSS_COMPILE: cross toolchain prefix by architecture
+CROSS_COMPILE ?= $(CROSS_COMPILE_$(XARCH))
+
+# make sure CC is prefixed with CROSS_COMPILE
+include ../../../scripts/Makefile.include

Thanks,
Zhangjin

> Willy