2019-09-10 12:42:11

by Ivan Khoronzhuk

[permalink] [raw]
Subject: [PATCH bpf-next 05/11] samples: bpf: makefile: use D vars from KBUILD_CFLAGS to handle headers

The kernel headers are reused from samples bpf, and autoconf.h is not
enough to reflect complete arch configuration for clang. But CLANG-bpf
cmds are sensitive for assembler part taken from linux headers and -D
vars, usually used in CFLAGS, should be carefully added for each arch.
For that, for CLANG-bpf, lets filter them only for arm arch as it
definitely requires __LINUX_ARM_ARCH__ to be set, but ignore for
others till it's really needed. For arm, -D__LINUX_ARM_ARCH__ is min
version used as instruction set selector. In another case errors
like "SMP is not supported" for arm and bunch of other errors are
issued resulting to incorrect final object.

Later D_OPTIONS can be used for gcc part.
---
samples/bpf/Makefile | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 8ecc5d0c2d5b..6492b7e65c08 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -185,6 +185,15 @@ HOSTLDLIBS_map_perf_test += -lrt
HOSTLDLIBS_test_overhead += -lrt
HOSTLDLIBS_xdpsock += -pthread

+# Strip all expet -D options needed to handle linux headers
+# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
+D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
+ sed '/^-D/!d' | tr '\n' ' ')
+
+ifeq ($(ARCH), arm)
+CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
+endif
+
# Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
# make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
LLC ?= llc
--
2.17.1


2019-09-13 21:14:56

by Yonghong Song

[permalink] [raw]
Subject: Re: [PATCH bpf-next 05/11] samples: bpf: makefile: use D vars from KBUILD_CFLAGS to handle headers



On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
> The kernel headers are reused from samples bpf, and autoconf.h is not
> enough to reflect complete arch configuration for clang. But CLANG-bpf
> cmds are sensitive for assembler part taken from linux headers and -D
> vars, usually used in CFLAGS, should be carefully added for each arch.
> For that, for CLANG-bpf, lets filter them only for arm arch as it
> definitely requires __LINUX_ARM_ARCH__ to be set, but ignore for
> others till it's really needed. For arm, -D__LINUX_ARM_ARCH__ is min
> version used as instruction set selector. In another case errors
> like "SMP is not supported" for arm and bunch of other errors are
> issued resulting to incorrect final object.
>
> Later D_OPTIONS can be used for gcc part.
> ---
> samples/bpf/Makefile | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 8ecc5d0c2d5b..6492b7e65c08 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -185,6 +185,15 @@ HOSTLDLIBS_map_perf_test += -lrt
> HOSTLDLIBS_test_overhead += -lrt
> HOSTLDLIBS_xdpsock += -pthread
>
> +# Strip all expet -D options needed to handle linux headers
> +# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
> +D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
> + sed '/^-D/!d' | tr '\n' ' ')
> +
> +ifeq ($(ARCH), arm)
> +CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
> +endif

Do you need this for native compilation?

so arm64 compilation does not need this?
If only -D__LINUX_ARM_ARCH__ is needed, maybe just
with
CLANG_EXTRA_CFLAGS := -D__LINUX_ARM_ARCH__
Otherwise, people will wonder whether this is needed for
other architectures. Or just do
CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
for all cross compilation?

> +
> # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
> # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> LLC ?= llc
>

2019-09-13 21:26:17

by Ivan Khoronzhuk

[permalink] [raw]
Subject: Re: [PATCH bpf-next 05/11] samples: bpf: makefile: use D vars from KBUILD_CFLAGS to handle headers

On Fri, Sep 13, 2019 at 09:12:01PM +0000, Yonghong Song wrote:
>
>
>On 9/10/19 11:38 AM, Ivan Khoronzhuk wrote:
>> The kernel headers are reused from samples bpf, and autoconf.h is not
>> enough to reflect complete arch configuration for clang. But CLANG-bpf
>> cmds are sensitive for assembler part taken from linux headers and -D
>> vars, usually used in CFLAGS, should be carefully added for each arch.
>> For that, for CLANG-bpf, lets filter them only for arm arch as it
>> definitely requires __LINUX_ARM_ARCH__ to be set, but ignore for
>> others till it's really needed. For arm, -D__LINUX_ARM_ARCH__ is min
>> version used as instruction set selector. In another case errors
>> like "SMP is not supported" for arm and bunch of other errors are
>> issued resulting to incorrect final object.
>>
>> Later D_OPTIONS can be used for gcc part.
>> ---
>> samples/bpf/Makefile | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
>> index 8ecc5d0c2d5b..6492b7e65c08 100644
>> --- a/samples/bpf/Makefile
>> +++ b/samples/bpf/Makefile
>> @@ -185,6 +185,15 @@ HOSTLDLIBS_map_perf_test += -lrt
>> HOSTLDLIBS_test_overhead += -lrt
>> HOSTLDLIBS_xdpsock += -pthread
>>
>> +# Strip all expet -D options needed to handle linux headers
>> +# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
>> +D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
>> + sed '/^-D/!d' | tr '\n' ' ')
>> +
>> +ifeq ($(ARCH), arm)
>> +CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>> +endif
>
>Do you need this for native compilation?
Yes, native "arm" also requires it.

>
>so arm64 compilation does not need this?
yes, now only arm

>If only -D__LINUX_ARM_ARCH__ is needed, maybe just
>with
> CLANG_EXTRA_CFLAGS := -D__LINUX_ARM_ARCH__
Value also needed: -D__LINUX_ARM_ARCH_=7 or -D__LINUX_ARM_ARCH_=6
So, need retrieve it.

>Otherwise, people will wonder whether this is needed for
>other architectures. Or just do
> CLANG_EXTRA_CFLAGS := $(D_OPTIONS)
>for all cross compilation?
arm, cross and native requires it.

Will do this:

# Strip all expet -D options needed to handle linux headers
# for arm it's __LINUX_ARM_ARCH__ and potentially others fork vars
ifeq ($(ARCH), arm)
D_OPTIONS = $(shell echo "$(KBUILD_CFLAGS) " | sed 's/[[:blank:]]/\n/g' | \
sed '/^-D/!d' | tr '\n' ' ')
endif

CLANG_EXTRA_CFLAGS := $(D_OPTIONS)



>
>> +
>> # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>> # make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
>> LLC ?= llc
>>

--
Regards,
Ivan Khoronzhuk