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.
