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

[Eduard Zingerman <eddyz87@gmail.com>]

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))
 #endif