BPF List
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Yonghong Song <yonghong.song@linux.dev>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	 Martin KaFai Lau <martin.lau@kernel.org>,
	Daniel Xu <dxu@dxuuu.xyz>,
	John Fastabend <john.fastabend@gmail.com>,
	bpf <bpf@vger.kernel.org>, Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 2/5] bpf: Introduce "volatile compare" macro
Date: Tue, 09 Jan 2024 01:16:58 +0200	[thread overview]
Message-ID: <c38e87c8301aa383ae622a14c75c39b2dc4f7989.camel@gmail.com> (raw)
In-Reply-To: <dc6ce5dc-afb9-4b5b-a99a-1577b99f6a96@linux.dev>

On Mon, 2024-01-08 at 13:21 -0800, Yonghong Song wrote:
[...]
> Thanks for the detailed analysis! Previously we intend to do the following:
> 
> - When 32-bit value is passed to "r" constraint:
>    - for cpu v3/v4 a 32-bit register should be selected;
>    - for cpu v1/v2 a warning should be reported.
> 
> So in the above, the desired asm code should be
> 
>      ...
>      # %bb.0:
>      	call bar
>      	#APP
>      	w0 += 1
>      	#NO_APP
>      	exit
>      ...
> 
> for cpuv3/cpuv4. I guess some more work in llvm is needed
> to achieve that.

To make clang emit w0 the following modification is needed:

diff --git a/llvm/lib/Target/BPF/BPFISelLowering.cpp b/llvm/lib/Target/BPF/BPFISelLowering.cpp
index b20e09c7f95f..4c504d587ce6 100644
--- a/llvm/lib/Target/BPF/BPFISelLowering.cpp
+++ b/llvm/lib/Target/BPF/BPFISelLowering.cpp
@@ -265,6 +265,8 @@ BPFTargetLowering::getRegForInlineAsmConstraint(const TargetRegisterInfo *TRI,
     // GCC Constraint Letters
     switch (Constraint[0]) {
     case 'r': // GENERAL_REGS
+      if (HasAlu32 && VT == MVT::i32)
+        return std::make_pair(0U, &BPF::GPR32RegClass);
       return std::make_pair(0U, &BPF::GPRRegClass);
     case 'w':
       if (HasAlu32)

However, as Alexei notes in the sibling thread,
this leads to incompatibility with some existing inline assembly.
E.g. there are two compilation errors in selftests.
I'll write in some more detail in the sibling thread.

> On the other hand, for cpuv3/v4, for regular C code,
> I think the compiler might be already omitting the conversion and use w
> register already. So I am not sure whether the patch [6]
> is needed or not. Could you double check?

Yes, for regular C code, generated assembly uses 32-bit registers as expected:

echo $(cat <<EOF
extern unsigned long bar(unsigned);
void foo(void) {
  bar(bar(7));
}
EOF
) | clang -mcpu=v3 -O2 --target=bpf -mcpu=v3 -x c -S -o - -

...

foo:                                    # @foo
# %bb.0:
	w1 = 7
	call bar
	w1 = w0
	call bar
	exit

  reply	other threads:[~2024-01-08 23:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21  3:38 [PATCH v2 bpf-next 0/5] bpf: volatile compare Alexei Starovoitov
2023-12-21  3:38 ` [PATCH v2 bpf-next 1/5] selftests/bpf: Attempt to build BPF programs with -Wsign-compare Alexei Starovoitov
2023-12-21  3:38 ` [PATCH v2 bpf-next 2/5] bpf: Introduce "volatile compare" macro Alexei Starovoitov
2023-12-21  4:27   ` Kumar Kartikeya Dwivedi
2023-12-22 22:59     ` Alexei Starovoitov
2023-12-25 20:33       ` Alexei Starovoitov
2024-01-04 20:06         ` Eduard Zingerman
2024-01-04 21:02           ` Alexei Starovoitov
2024-01-05 21:47         ` Eduard Zingerman
2024-01-08 21:21           ` Yonghong Song
2024-01-08 23:16             ` Eduard Zingerman [this message]
2024-01-08 21:33           ` asm register constraint. Was: " Alexei Starovoitov
2024-01-08 23:22             ` Yonghong Song
2024-01-09 10:49             ` Jose E. Marchesi
2024-01-09 12:09               ` Jose E. Marchesi
2024-01-11 18:33                 ` Jose E. Marchesi
2024-01-15 16:33                   ` Eduard Zingerman
2024-01-16 17:47                     ` Alexei Starovoitov
2024-01-16 19:07                       ` Yonghong Song
2024-01-16 19:34                         ` Alexei Starovoitov
2024-01-16 23:14                           ` Yonghong Song
2024-01-17 22:43                             ` Alexei Starovoitov
2024-01-16 23:55                       ` Eduard Zingerman
2024-01-16 18:40                     ` Alexei Starovoitov
2024-01-16 23:15                       ` Eduard Zingerman
2024-01-11  2:46               ` Alexei Starovoitov
2024-01-11 10:34                 ` Jose E. Marchesi
2024-01-11 16:46             ` Eduard Zingerman
2023-12-21  3:38 ` [PATCH v2 bpf-next 3/5] selftests/bpf: Convert exceptions_assert.c to bpf_cmp Alexei Starovoitov
2023-12-21  3:38 ` [PATCH v2 bpf-next 4/5] selftests/bpf: Remove bpf_assert_eq-like macros Alexei Starovoitov
2023-12-21  3:38 ` [RFC PATCH v2 bpf-next 5/5] selftests/bpf: Attempt to convert profiler.c to bpf_cmp Alexei Starovoitov
2023-12-21 18:01 ` [PATCH v2 bpf-next 0/5] bpf: volatile compare Jiri Olsa

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=c38e87c8301aa383ae622a14c75c39b2dc4f7989.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dxu@dxuuu.xyz \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=yonghong.song@linux.dev \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox