On Mon Dec 16, 2024 at 8:47 PM CET, Alexei Starovoitov wrote: > On Mon, Dec 16, 2024 at 10:50 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > On Mon, 2024-12-16 at 10:05 -0800, Alexei Starovoitov wrote: > > > > [...] > > > > > > Thanks for the review! Good point, I'll try to write them in C. > > > > > > > > It might not be possible to do them both entirely: clang also doesn't > > > > know that bpf_tail_call() can return, so it assumes the callee() will > > > > return a constant r0. It sometimes optimizes branches / loads out > > > > because of this. > > > > > > I wonder whether we should tell llvm that it's similar to longjmp() > > > with __attribute__((noreturn)) or some other attribute. > > > > GCC documents it as follows [1]: > > > > > The noreturn keyword tells the compiler to assume that fatal > > > cannot reaturn. It can then optimize without regard to what would > > > happen if fatal ever did return. This makes slightly better code. > > > More importantly, it helps avoid spurious warnings of > > > uninitialized variables. > > > > But the bpf_tail_call could return if MAX_TAIL_CALL_CNT limit is exceeded, > > or programs map index is out of bounds. > > > > [1] https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute > > Yeah. noreturn is too heavy. > attr(returns_twice) is another option, but it will probably > pessimize the code too much as well. We could provide a macro like: #define BPF_TAIL_CALL(ctx, map, index) do { \ bpf_tail_call(ctx, map, index); \ int maybe_return; \ asm volatile("%0 = 0" : "=r"(maybe_return)); \ if (maybe_return) \ return maybe_return; \ } while(0) It correctly tricks clang into thinking it can return early, but the verifier removes the dead branch so there's no runtime cost. But users would have to switch to it, I don't think we can replace the definition in bpf_helper_defs.h in a backwards compatible way.
