From: "Björn Töpel" <bjorn.topel@gmail.com> To: Palmer Dabbelt <palmerdabbelt@google.com> Cc: Daniel Borkmann <daniel@iogearbox.net>, Alexei Starovoitov <ast@kernel.org>, zlim.lnx@gmail.com, catalin.marinas@arm.com, will@kernel.org, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>, Shuah Khan <shuah@kernel.org>, Netdev <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>, linux-arm-kernel@lists.infradead.org, LKML <linux-kernel@vger.kernel.org>, linux-kselftest@vger.kernel.org, clang-built-linux@googlegroups.com, kernel-team@android.com Subject: Re: [PATCH 4/4] arm64: bpf: Elide some moves to a0 after calls Date: Tue, 4 Feb 2020 20:13:17 +0100 [thread overview] Message-ID: <CAJ+HfNjkacY-KStgGJMgvQh2=2OsMnH6Saij+nAPBqQrSJcNWw@mail.gmail.com> (raw) In-Reply-To: <20200128021145.36774-5-palmerdabbelt@google.com> On Tue, 28 Jan 2020 at 03:15, Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On arm64, the BPF function ABI doesn't match the C function ABI. Specifically, > arm64 encodes calls as `a0 = f(a0, a1, ...)` while BPF encodes calls as > `BPF_REG_0 = f(BPF_REG_1, BPF_REG_2, ...)`. This discrepancy results in > function calls being encoded as a two operations sequence that first does a C > ABI calls and then moves the return register into the right place. This > results in one extra instruction for every function call. > It's a lot of extra work for one reg-to-reg move, but it always annoyed me in the RISC-V JIT. :-) So, if it *can* be avoided, why not. [...] > > +static int dead_register(const struct jit_ctx *ctx, int offset, int bpf_reg) Given that a lot of archs (RISC-V, arm?, MIPS?) might benefit from this, it would be nice if it could be made generic (it already is pretty much), and moved to kernel/bpf. > +{ > + const struct bpf_prog *prog = ctx->prog; > + int i; > + > + for (i = offset; i < prog->len; ++i) { > + const struct bpf_insn *insn = &prog->insnsi[i]; > + const u8 code = insn->code; > + const u8 bpf_dst = insn->dst_reg; > + const u8 bpf_src = insn->src_reg; > + const int writes_dst = !((code & BPF_ST) || (code & BPF_STX) > + || (code & BPF_JMP32) || (code & BPF_JMP)); > + const int reads_dst = !((code & BPF_LD)); > + const int reads_src = true; > + > + /* Calls are a bit special in that they clobber a bunch of regisers. */ > + if ((code & (BPF_JMP | BPF_CALL)) || (code & (BPF_JMP | BPF_TAIL_CALL))) > + if ((bpf_reg >= BPF_REG_0) && (bpf_reg <= BPF_REG_5)) > + return false; > + > + /* Registers that are read before they're written are alive. > + * Most opcodes are of the form DST = DEST op SRC, but there > + * are some exceptions.*/ > + if (bpf_src == bpf_reg && reads_src) > + return false; > + > + if (bpf_dst == bpf_reg && reads_dst) > + return false; > + > + if (bpf_dst == bpf_reg && writes_dst) > + return true; > + > + /* Most BPF instructions are 8 bits long, but some ar 16 bits > + * long. */ A bunch of spelling errors above. Cheers, Björn
WARNING: multiple messages have this Message-ID (diff)
From: "Björn Töpel" <bjorn.topel@gmail.com> To: Palmer Dabbelt <palmerdabbelt@google.com> Cc: Song Liu <songliubraving@fb.com>, Andrii Nakryiko <andriin@fb.com>, Daniel Borkmann <daniel@iogearbox.net>, kernel-team@android.com, zlim.lnx@gmail.com, Shuah Khan <shuah@kernel.org>, Alexei Starovoitov <ast@kernel.org>, LKML <linux-kernel@vger.kernel.org>, clang-built-linux@googlegroups.com, Netdev <netdev@vger.kernel.org>, linux-kselftest@vger.kernel.org, catalin.marinas@arm.com, Yonghong Song <yhs@fb.com>, bpf <bpf@vger.kernel.org>, will@kernel.org, Martin KaFai Lau <kafai@fb.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 4/4] arm64: bpf: Elide some moves to a0 after calls Date: Tue, 4 Feb 2020 20:13:17 +0100 [thread overview] Message-ID: <CAJ+HfNjkacY-KStgGJMgvQh2=2OsMnH6Saij+nAPBqQrSJcNWw@mail.gmail.com> (raw) In-Reply-To: <20200128021145.36774-5-palmerdabbelt@google.com> On Tue, 28 Jan 2020 at 03:15, Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On arm64, the BPF function ABI doesn't match the C function ABI. Specifically, > arm64 encodes calls as `a0 = f(a0, a1, ...)` while BPF encodes calls as > `BPF_REG_0 = f(BPF_REG_1, BPF_REG_2, ...)`. This discrepancy results in > function calls being encoded as a two operations sequence that first does a C > ABI calls and then moves the return register into the right place. This > results in one extra instruction for every function call. > It's a lot of extra work for one reg-to-reg move, but it always annoyed me in the RISC-V JIT. :-) So, if it *can* be avoided, why not. [...] > > +static int dead_register(const struct jit_ctx *ctx, int offset, int bpf_reg) Given that a lot of archs (RISC-V, arm?, MIPS?) might benefit from this, it would be nice if it could be made generic (it already is pretty much), and moved to kernel/bpf. > +{ > + const struct bpf_prog *prog = ctx->prog; > + int i; > + > + for (i = offset; i < prog->len; ++i) { > + const struct bpf_insn *insn = &prog->insnsi[i]; > + const u8 code = insn->code; > + const u8 bpf_dst = insn->dst_reg; > + const u8 bpf_src = insn->src_reg; > + const int writes_dst = !((code & BPF_ST) || (code & BPF_STX) > + || (code & BPF_JMP32) || (code & BPF_JMP)); > + const int reads_dst = !((code & BPF_LD)); > + const int reads_src = true; > + > + /* Calls are a bit special in that they clobber a bunch of regisers. */ > + if ((code & (BPF_JMP | BPF_CALL)) || (code & (BPF_JMP | BPF_TAIL_CALL))) > + if ((bpf_reg >= BPF_REG_0) && (bpf_reg <= BPF_REG_5)) > + return false; > + > + /* Registers that are read before they're written are alive. > + * Most opcodes are of the form DST = DEST op SRC, but there > + * are some exceptions.*/ > + if (bpf_src == bpf_reg && reads_src) > + return false; > + > + if (bpf_dst == bpf_reg && reads_dst) > + return false; > + > + if (bpf_dst == bpf_reg && writes_dst) > + return true; > + > + /* Most BPF instructions are 8 bits long, but some ar 16 bits > + * long. */ A bunch of spelling errors above. Cheers, Björn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-02-04 19:13 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-28 2:11 arm64: bpf: Elide some moves to a0 after calls Palmer Dabbelt 2020-01-28 2:11 ` Palmer Dabbelt 2020-01-28 2:11 ` [PATCH 1/4] selftests/bpf: Elide a check for LLVM versions that can't compile it Palmer Dabbelt 2020-01-28 2:11 ` Palmer Dabbelt 2020-02-11 18:20 ` Nick Desaulniers 2020-02-11 18:20 ` Nick Desaulniers 2020-01-28 2:11 ` [PATCH 2/4] arm64: bpf: Convert bpf2a64 to a function Palmer Dabbelt 2020-01-28 2:11 ` Palmer Dabbelt 2020-01-28 2:11 ` [PATCH 3/4] arm64: bpf: Split the read and write halves of dst Palmer Dabbelt 2020-01-28 2:11 ` Palmer Dabbelt 2020-01-28 2:11 ` [PATCH 4/4] arm64: bpf: Elide some moves to a0 after calls Palmer Dabbelt 2020-01-28 2:11 ` Palmer Dabbelt 2020-02-04 19:13 ` Björn Töpel [this message] 2020-02-04 19:13 ` Björn Töpel 2020-02-11 0:15 ` Alexei Starovoitov 2020-02-11 0:15 ` Alexei Starovoitov 2020-02-04 19:30 ` Björn Töpel 2020-02-04 19:30 ` Björn Töpel 2020-02-04 20:33 ` John Fastabend 2020-02-04 20:33 ` John Fastabend 2020-02-18 19:28 ` Palmer Dabbelt 2020-02-18 19:28 ` Palmer Dabbelt
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAJ+HfNjkacY-KStgGJMgvQh2=2OsMnH6Saij+nAPBqQrSJcNWw@mail.gmail.com' \ --to=bjorn.topel@gmail.com \ --cc=andriin@fb.com \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=catalin.marinas@arm.com \ --cc=clang-built-linux@googlegroups.com \ --cc=daniel@iogearbox.net \ --cc=kafai@fb.com \ --cc=kernel-team@android.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-kselftest@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=palmerdabbelt@google.com \ --cc=shuah@kernel.org \ --cc=songliubraving@fb.com \ --cc=will@kernel.org \ --cc=yhs@fb.com \ --cc=zlim.lnx@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.