Re: [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc

[Yonghong Song <yhs@fb.com>]

On 12/9/20 12:53 PM, Andrew Delgadillo wrote:
> LLC is meant for compiler development and debugging. Consequently, it
> exposes many low level options about its backend. To avoid future bugs
> introduced by using the raw LLC tool, use clang directly so that all
> appropriate options are passed to the back end.

Agree that this indeed make build system simpler.

> 
> Additionally, the native clang build rule was not being use in the
> selftests Makefile, so remove it.

This is true too. Otherwise, native clang build will require both
clang and llc runs.

The patch looks good and I have a few comments and hopefully
you can accommodate.

> 
> Signed-off-by: Andrew Delgadillo <adelg@google.com>
> ---
>   tools/testing/selftests/bpf/Makefile | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 944ae17a39ed..74870d365b62 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -19,7 +19,6 @@ ifneq ($(wildcard $(GENHDR)),)
>   endif
>   
>   CLANG		?= clang
> -LLC		?= llc
>   LLVM_OBJCOPY	?= llvm-objcopy
>   BPF_GCC		?= $(shell command -v bpf-gcc;)
>   SAN_CFLAGS	?=
> @@ -256,24 +255,13 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
>   # $3 - CFLAGS
>   # $4 - LDFLAGS
>   define CLANG_BPF_BUILD_RULE
> -	$(call msg,CLNG-LLC,$(TRUNNER_BINARY),$2)
> -	$(Q)($(CLANG) $3 -O2 -target bpf -emit-llvm			\
> -		-c $1 -o - || echo "BPF obj compilation failed") | 	\
> -	$(LLC) -mattr=dwarfris -march=bpf -mcpu=v3 $4 -filetype=obj -o $2
> +	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> +	$(Q)$(CLANG) $3 -O2 -target bpf -c $1 -o $2 -Xclang -target-feature -Xclang +dwarfris -mcpu=v3 $4

Yes, we still use +dwarfris here.
The original llvm patch which introduded +dwarfris is:
 
https://github.com/llvm/llvm-project/commit/03e1c8b8f9cc7b898217b7789d3887a903443c93
it is to workaround an elfutils/libdw issue as it does not support bpf 
backend so pahole cannot display debuginfo structures properly.
Subsequently, the elfutils/libdw bpf support is added at
 
https://sourceware.org/git/?p=elfutils.git;a=commitdiff;h=c1990d36cfe37a30bcc49422c37a6767fd190559

Any recent pahole should already build with the above fix.
I tested with pahole 1.16 it works fine for binaries built without
+dwarfris. Also BTF now can be used to dump structures.

So could you also accommodate the change to remove +dwarfris option?


>   endef
>   # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32
>   define CLANG_NOALU32_BPF_BUILD_RULE
> -	$(call msg,CLNG-LLC,$(TRUNNER_BINARY),$2)
> -	$(Q)($(CLANG) $3 -O2 -target bpf -emit-llvm			\
> -		-c $1 -o - || echo "BPF obj compilation failed") | 	\
> -	$(LLC) -march=bpf -mcpu=v2 $4 -filetype=obj -o $2
> -endef
> -# Similar to CLANG_BPF_BUILD_RULE, but using native Clang and bpf LLC
> -define CLANG_NATIVE_BPF_BUILD_RULE
>   	$(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2)
> -	$(Q)($(CLANG) $3 -O2 -emit-llvm					\
> -		-c $1 -o - || echo "BPF obj compilation failed") | 	\
> -	$(LLC) -march=bpf -mcpu=v3 $4 -filetype=obj -o $2
> +	$(Q)$(CLANG) $3 -O2 -target bpf -c $1 -o $2 -mcpu=v2 $4
>   endef
>   # Build BPF object using GCC
>   define GCC_BPF_BUILD_RULE
> @@ -402,7 +390,7 @@ TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko	\
>   		       $(wildcard progs/btf_dump_test_case_*.c)
>   TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
>   TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
> -TRUNNER_BPF_LDFLAGS := -mattr=+alu32
> +TRUNNER_BPF_LDFLAGS := -Xclang -target-feature -Xclang +alu32

The +alu32 is only used for non-alu32 case where -mcpu=v3 actually 
implies alu32. So let us remove TRUNNER_BPF_LDFLAGS flag from Makefile too.

>   $(eval $(call DEFINE_TEST_RUNNER,test_progs))
>   
>   # Define test_progs-no_alu32 test runner.
>