Re: [PATCH] bpf: Allow small structs to be type of function argument

[John Fastabend <john.fastabend@gmail.com>]

Yonghong Song wrote:
> 
> 
> On 6/18/20 7:04 PM, Alexei Starovoitov wrote:
> > On Thu, Jun 18, 2020 at 5:26 PM John Fastabend <john.fastabend@gmail.com> wrote:
> >>
> >>   foo(int a, __int128 b)
> >>
> >> would put a in r0 and b in r2 and r3 leaving a hole in r1. But that
> >> was some old reference manual and  might no longer be the case
> 
> This should not happen if clang compilation with -target bpf.
> This MAY happen if they compile with 'clang -target riscv' as the IR
> could change before coming to bpf backend.

I guess this means in order to handle __int128 and structs in
btf_ctx_access we would have to know this did not happen. Otherwise
the arg to type mappings are off because we simply do

 arg = off / 8

> 
> >> in reality. Perhaps just spreading hearsay, but the point is we
> >> should say something about what the BPF backend convention
> >> is and write it down. We've started to bump into these things
> >> lately.
> > 
> > calling convention for int128 in bpf is _undefined_.
> > calling convention for struct by value in bpf is also _undefined_.
> 
> Just to clarify a little bit. bpf backend did not do anything
> special about int128 and struct type. It is using llvm default.
> That is, int128 using two argument registers and struct passed
> by address. But I do see some other architectures having their
> own ways to handle these parameters like X86, AARCH64, AMDGPU, MIPS.
> 
> int128 is not widely used. passing struct as the argument is not
> a good practice. So Agree with Alexei is not really worthwhile to
> handle them in the place of arguments.

Agree as well I'll just add a small fix to check btf_type_is_int()
size is <= u64 and that should be sufficient to skip handling the
int128 case.

> 
> > 
> > In many cases the compiler has to have the backend code
> > so other parts of the compiler can function.
> > I didn't bother explicitly disabling every undefined case.
> > Please don't read too much into llvm generated code.
> >