Messages in this thread | | | Date | Wed, 11 Apr 2018 15:39:02 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: bpf: handling non BPF register names in inline assembly with -target bpf |
| |
Em Wed, Apr 11, 2018 at 09:37:46AM -0700, Yonghong Song escreveu: > Hi, Arnaldo,
> When I studied the bpf compilation issue with latest linus/net-next > kernel (https://patchwork.kernel.org/patch/10333829/), an alternative > approach I tried is to use __BPF__ macro.
You mean you used an alternative approach that does _not_ use the __BPF__ macro, right? I looked at the patch and yeah, looks sane as well, since the kernel build process already defines that CC_HAVE_ASM_GOTO, checking if gcc has that feature, etc.
> The following patch introduced "#ifndef __BPF__" in > arch/x86/include/asm/asm.h for some inline assembly related to x86 > "esp" register name.
> ========== > commit ca26cffa4e4aaeb09bb9e308f95c7835cb149248 > Author: Arnaldo Carvalho de Melo <acme@redhat.com> > Date: Mon Dec 4 13:08:47 2017 -0300 > > x86/asm: Allow again using asm.h when building for the 'bpf' > clang target > > Up to f5caf621ee35 ("x86/asm: Fix inline asm call constraints > for Clang") > we were able to use x86 headers to build to the 'bpf' clang target, as > done by the BPF code in tools/perf/. > ... > diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h > index 219faae..386a690 100644 > --- a/arch/x86/include/asm/asm.h > +++ b/arch/x86/include/asm/asm.h > @@ -136,6 +136,7 @@ > #endif > > #ifndef __ASSEMBLY__ > +#ifndef __BPF__ > /* > * This output constraint should be used for any inline asm which > has a "call" > * instruction. Otherwise the asm may be inserted before the frame > pointer > @@ -145,5 +146,6 @@ > register unsigned long current_stack_pointer asm(_ASM_SP); > #define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer) > #endif > +#endif > ... > ========== > > I just landed a clang patch (clang 7.0.0 trunk) > https://reviews.llvm.org/rL329823 > which will permit bpf clang target to accept ANY register > names. In this case, the inline assembly will be accepted by clang > and will be thrown away since variable current_stack_pointer is > not used in bpf programs.
Ok, then that ifndef __BPF__ above will not be needed anymore, but only people with clang > that version will be able to build tools/perf/ > If the inline assembly is indeed for BPF program, later llc > AsmParser will do syntax and semantics checking again. > > With the above clang patch, the above "#ifndef __BPF__" can be removed. > You can decide when is the appropriate time to use latest clang compiler > and remove the above "#ifndef __BPF__".
So are you proposing that we have something similar to that CC_HAVE_ASM_GOTO check in the kernel main Makefile, to define something like CC_HAVE_ASM_REGS (or some better name), i.e. something like:
# check for 'asm(_ASM_SP)' ifeq ($(call shell-cached,$(CONFIG_SHELL) $(srctree)/scripts/cc-asm-regs.sh $(CC) $(KBUILD_CFLAGS)), y) CC_HAVE_ASM_REGS := 1 KBUILD_CFLAGS += -DCC_HAVE_ASM_REGS KBUILD_AFLAGS += -DCC_HAVE_ASM_REGS endif
?
- Arnaldo
| |