All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc
@ 2020-12-09 20:53 Andrew Delgadillo
  2020-12-10  2:16 ` Yonghong Song
  2020-12-10 19:41 ` [PATCH bpf-next v2] " Andrew Delgadillo
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Delgadillo @ 2020-12-09 20:53 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrew Delgadillo

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.

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

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
 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
 $(eval $(call DEFINE_TEST_RUNNER,test_progs))
 
 # Define test_progs-no_alu32 test runner.
-- 
2.29.2.576.ga3fc446d84-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc
  2020-12-09 20:53 [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc Andrew Delgadillo
@ 2020-12-10  2:16 ` Yonghong Song
  2020-12-10 17:12   ` Andrew Delgadillo
  2020-12-10 19:41 ` [PATCH bpf-next v2] " Andrew Delgadillo
  1 sibling, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2020-12-10  2:16 UTC (permalink / raw)
  To: Andrew Delgadillo, bpf; +Cc: Alexei Starovoitov, Daniel Borkmann



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.
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc
  2020-12-10  2:16 ` Yonghong Song
@ 2020-12-10 17:12   ` Andrew Delgadillo
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Delgadillo @ 2020-12-10 17:12 UTC (permalink / raw)
  To: Yonghong Song; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v2] selftests/bpf: Drop the need for LLVM's llc
  2020-12-09 20:53 [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc Andrew Delgadillo
  2020-12-10  2:16 ` Yonghong Song
@ 2020-12-10 19:41 ` Andrew Delgadillo
  2020-12-10 19:41   ` Andrew Delgadillo
  1 sibling, 1 reply; 9+ messages in thread
From: Andrew Delgadillo @ 2020-12-10 19:41 UTC (permalink / raw)
  To: bpf, Alexei Starovoitov, daniel, yhs; +Cc: adelg

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.

Additionally, simplify the Makefile by removing the
CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
dwarfris attr since elfutils/libdw now supports the bpf backend (which
should work with any recent pahole), and stop passing alu32 since
-mcpu=v3 implies alu32.

Signed-off-by: Andrew Delgadillo <adelg@google.com>
---
Changes since v1:
* do not pass +dwarfris
* do not pass +alu32 when using -mcpu=v3
---
 tools/testing/selftests/bpf/Makefile | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 944ae17a39ed..a96f63dfd8dc 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 -mcpu=v3 $4
 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,6 @@ 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
 $(eval $(call DEFINE_TEST_RUNNER,test_progs))
 
 # Define test_progs-no_alu32 test runner.
-- 
2.29.2.576.ga3fc446d84-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v2] selftests/bpf: Drop the need for LLVM's llc
  2020-12-10 19:41 ` [PATCH bpf-next v2] " Andrew Delgadillo
@ 2020-12-10 19:41   ` Andrew Delgadillo
  2020-12-10 23:55     ` Yonghong Song
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Delgadillo @ 2020-12-10 19:41 UTC (permalink / raw)
  To: bpf, Alexei Starovoitov, daniel, yhs; +Cc: adelg

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.

Additionally, simplify the Makefile by removing the
CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
dwarfris attr since elfutils/libdw now supports the bpf backend (which
should work with any recent pahole), and stop passing alu32 since
-mcpu=v3 implies alu32.

Signed-off-by: Andrew Delgadillo <adelg@google.com>
---
Changes since v1:
* do not pass +dwarfris
* do not pass +alu32 when using -mcpu=v3
---
 tools/testing/selftests/bpf/Makefile | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 944ae17a39ed..a96f63dfd8dc 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 -mcpu=v3 $4
 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,6 @@ 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
 $(eval $(call DEFINE_TEST_RUNNER,test_progs))
 
 # Define test_progs-no_alu32 test runner.
-- 
2.29.2.576.ga3fc446d84-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next v2] selftests/bpf: Drop the need for LLVM's llc
  2020-12-10 19:41   ` Andrew Delgadillo
@ 2020-12-10 23:55     ` Yonghong Song
  2020-12-11  0:43       ` [PATCH bpf-next v3] " Andrew Delgadillo
  0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2020-12-10 23:55 UTC (permalink / raw)
  To: Andrew Delgadillo, bpf, Alexei Starovoitov, daniel



On 12/10/20 11:41 AM, 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.
> 
> Additionally, simplify the Makefile by removing the
> CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
> dwarfris attr since elfutils/libdw now supports the bpf backend (which
> should work with any recent pahole), and stop passing alu32 since
> -mcpu=v3 implies alu32.
> 
> Signed-off-by: Andrew Delgadillo <adelg@google.com>
> ---
> Changes since v1:
> * do not pass +dwarfris
> * do not pass +alu32 when using -mcpu=v3
> ---
>   tools/testing/selftests/bpf/Makefile | 19 +++----------------
>   1 file changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 944ae17a39ed..a96f63dfd8dc 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

You can remove TRUNNER_BPF_LDFLAGS completely, so we won't have $4 here.

>   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 -mcpu=v3 $4

and $4 here.

>   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

and $4 here.

>   endef
>   # Build BPF object using GCC
>   define GCC_BPF_BUILD_RULE
> @@ -402,7 +390,6 @@ 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
>   $(eval $(call DEFINE_TEST_RUNNER,test_progs))
>   
>   # Define test_progs-no_alu32 test runner.
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH bpf-next v3] selftests/bpf: Drop the need for LLVM's llc
  2020-12-10 23:55     ` Yonghong Song
@ 2020-12-11  0:43       ` Andrew Delgadillo
  2020-12-11  1:26         ` Yonghong Song
  2020-12-11  6:10         ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Delgadillo @ 2020-12-11  0:43 UTC (permalink / raw)
  To: Yonghong Song, bpf, Alexei Starovoitov, daniel; +Cc: Andrew Delgadillo

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.

Additionally, simplify the Makefile by removing the
CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
dwarfris attr since elfutils/libdw now supports the bpf backend (which
should work with any recent pahole), and stop passing alu32 since
-mcpu=v3 implies alu32.

Signed-off-by: Andrew Delgadillo <adelg@google.com>
---
Changes since v2:
* Remove TRUNNER_BPF_LDFLAGS since they are not used
Changes since v1:
* do not pass +dwarfris
* do not pass +alu32 when using -mcpu=v3
---
 tools/testing/selftests/bpf/Makefile | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 944ae17a39ed..70122f414bcd 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	?=
@@ -254,31 +253,19 @@ $(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
 # $1 - input .c file
 # $2 - output .o file
 # $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 -mcpu=v3
 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
 endef
 # Build BPF object using GCC
 define GCC_BPF_BUILD_RULE
 	$(call msg,GCC-BPF,$(TRUNNER_BINARY),$2)
-	$(Q)$(BPF_GCC) $3 $4 -O2 -c $1 -o $2
+	$(Q)$(BPF_GCC) $3 -O2 -c $1 -o $2
 endef
 
 SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
@@ -333,8 +320,7 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
 		     $$(INCLUDE_DIR)/vmlinux.h				\
 		     $(wildcard $(BPFDIR)/bpf_*.h) | $(TRUNNER_OUTPUT)
 	$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,			\
-					  $(TRUNNER_BPF_CFLAGS),	\
-					  $(TRUNNER_BPF_LDFLAGS))
+					  $(TRUNNER_BPF_CFLAGS))
 
 $(TRUNNER_BPF_SKELS): $(TRUNNER_OUTPUT)/%.skel.h:			\
 		      $(TRUNNER_OUTPUT)/%.o				\
@@ -402,19 +388,16 @@ 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
 $(eval $(call DEFINE_TEST_RUNNER,test_progs))
 
 # Define test_progs-no_alu32 test runner.
 TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE
-TRUNNER_BPF_LDFLAGS :=
 $(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32))
 
 # Define test_progs BPF-GCC-flavored test runner.
 ifneq ($(BPF_GCC),)
 TRUNNER_BPF_BUILD_RULE := GCC_BPF_BUILD_RULE
 TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(call get_sys_includes,gcc)
-TRUNNER_BPF_LDFLAGS :=
 $(eval $(call DEFINE_TEST_RUNNER,test_progs,bpf_gcc))
 endif
 
@@ -425,7 +408,6 @@ TRUNNER_EXTRA_SOURCES := test_maps.c
 TRUNNER_EXTRA_FILES :=
 TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
 TRUNNER_BPF_CFLAGS :=
-TRUNNER_BPF_LDFLAGS :=
 $(eval $(call DEFINE_TEST_RUNNER,test_maps))
 
 # Define test_verifier test runner.
-- 
2.29.2.576.ga3fc446d84-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next v3] selftests/bpf: Drop the need for LLVM's llc
  2020-12-11  0:43       ` [PATCH bpf-next v3] " Andrew Delgadillo
@ 2020-12-11  1:26         ` Yonghong Song
  2020-12-11  6:10         ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2020-12-11  1:26 UTC (permalink / raw)
  To: Andrew Delgadillo, bpf, Alexei Starovoitov, daniel



On 12/10/20 4:43 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.
> 
> Additionally, simplify the Makefile by removing the
> CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
> dwarfris attr since elfutils/libdw now supports the bpf backend (which
> should work with any recent pahole), and stop passing alu32 since
> -mcpu=v3 implies alu32.
> 
> Signed-off-by: Andrew Delgadillo <adelg@google.com>

Acked-by: Yonghong Song <yhs@fb.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH bpf-next v3] selftests/bpf: Drop the need for LLVM's llc
  2020-12-11  0:43       ` [PATCH bpf-next v3] " Andrew Delgadillo
  2020-12-11  1:26         ` Yonghong Song
@ 2020-12-11  6:10         ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2020-12-11  6:10 UTC (permalink / raw)
  To: Andrew Delgadillo; +Cc: yhs, bpf, ast, daniel

Hello:

This patch was applied to bpf/bpf-next.git (refs/heads/master):

On Fri, 11 Dec 2020 00:43:44 +0000 you 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.
> 
> Additionally, simplify the Makefile by removing the
> CLANG_NATIVE_BPF_BUILD_RULE as it is not being use, stop passing
> dwarfris attr since elfutils/libdw now supports the bpf backend (which
> should work with any recent pahole), and stop passing alu32 since
> -mcpu=v3 implies alu32.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3] selftests/bpf: Drop the need for LLVM's llc
    https://git.kernel.org/bpf/bpf-next/c/89ad7420b25c

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-11  6:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-09 20:53 [PATCH bpf-next] selftests/bpf: Drop the need for LLVM's llc Andrew Delgadillo
2020-12-10  2:16 ` Yonghong Song
2020-12-10 17:12   ` Andrew Delgadillo
2020-12-10 19:41 ` [PATCH bpf-next v2] " Andrew Delgadillo
2020-12-10 19:41   ` Andrew Delgadillo
2020-12-10 23:55     ` Yonghong Song
2020-12-11  0:43       ` [PATCH bpf-next v3] " Andrew Delgadillo
2020-12-11  1:26         ` Yonghong Song
2020-12-11  6:10         ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.