Re: [PATCH v2 bpf-next 16/20] bpf: Add helper macro bpf_arena_cast()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 14, 2024 at 8:47 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Tue, 2024-02-13 at 14:35 -0800, Alexei Starovoitov wrote:
> [...]
>
> > This arena bpf_arena_cast() macro probably will be removed
> > once llvm 19 is released and we upgrade bpf CI to it.
> > It's here for selftests only.
> > It's quite tricky and fragile to use in practice.
> > Notice it does:
> > "r"(__var)
> > which is not quite correct,
> > since llvm won't recognize it as output that changes __var and
> > will use a copy of __var in a different register later.
> > But if the macro changes to "=r" or "+r" then llvm allocates
> > a register and that screws up codegen even more.
> >
> > The __var;}) also doesn't always work.
> > So this macro is not suited for all to use.
>
> Could you please elaborate a bit on why is this macro fragile?
> I toyed a bit with a version patched as below and it seems to work fine.
> Don't see how  ": [reg]"+r"(var) : ..." could be broken by the compiler
> (when "+r" is in the "output constraint" position):
> from clang pov the variable 'var' would be in register and updated
> after the asm volatile part.
>
> ---
>
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index e73b7d48439f..488001236506 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -334,8 +334,6 @@ l_true:                                                                                             \
>  /* emit instruction: rX=rX .off = mode .imm32 = address_space */
>  #ifndef bpf_arena_cast
>  #define bpf_arena_cast(var, mode, addr_space)  \
> -       ({                                      \
> -       typeof(var) __var = var;                \
>         asm volatile(".byte 0xBF;               \
>                      .ifc %[reg], r0;           \
>                      .byte 0x00;                \
> @@ -368,8 +366,7 @@ l_true:                                                                                             \
>                      .byte 0x99;                \
>                      .endif;                    \
>                      .short %[off]; .long %[as]"        \
> -                    :: [reg]"r"(__var), [off]"i"(mode), [as]"i"(addr_space)); __var; \
> -       })
> +                    : [reg]"+r"(var) : [off]"i"(mode), [as]"i"(addr_space))

Earlier I tried "+r" while keeping __var.
Directly using var seems to work indeed.
I'll apply this change.





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux