On Wed, Dec 9, 2020 at 6:16 PM Yonghong Song <yhs@xxxxxx> wrote: > > > > 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@xxxxxxxxxx> > > --- > > 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. > > Thank you for reviewing. I will make those changes and send a v2 of the patch.
