[PATCH] D72184: [BPF] support atomic instructions

[Brendan Jackman via Phabricator via cfe-commits]

jackmanb added a comment.

Sorry I was disrupted and not able to work on this last week! I've just got started trying to integrate this with my kernel patches.



================
Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:666
+    def XADDD : XADD<BPF_DW, "u64", atomic_load_add_64>;
+  }
+}
----------------
FYI - I just spotted some stray `\t` in here (is it helpful to point this out? If not let me know, I will ignore in future)


================
Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:684
+  let Inst{47-32} = addr{15-0}; // offset
+  let Inst{11-8} = val;
+  let Inst{7-4} = Opc.Value;
----------------
Sorry I'm a beginner with the LLVM code, could you explain what `val` does? I didn't notice this when I looked through here before.

To try and get a clue I tried just removing this line and then compiling the following code:

```C
// SPDX-License-Identifier: GPL-2.0
#include <stdatomic.h>

#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>

__u64 test_data_64 = 0;
__u64 test1_result = 0;

SEC("fentry/bpf_fentry_test1")
int BPF_PROG(test1, int a)
{
        /* atomic_fetch_add(&test_data_64, 1); */
        test1_result = __sync_fetch_and_add(&test_data_64, 1);
        return 0;
}
```

And I was able to load and run the program, with the kernel on my WIP branch: https://github.com/bjackman/linux-bpf/tree/wips/bpf-atomics-v0

The result looks like this:

```shell
$ llvm-objdump -d atomics_test.o

atomics_test.o: file format elf64-bpf


Disassembly of section fentry/bpf_fentry_test1:

0000000000000000 <test1>:
       0:       b7 01 00 00 01 00 00 00 r1 = 1
       1:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
       3:       db 12 00 00 01 00 00 00 r1 = atomic_fetch_add((u64 *)(r2 + 0), PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.      Program arguments: llvm-objdump -d atomics_test.o 
Segmentation fault
```

Aside from the fact that llvm-objdump crashed, the encoding `db 12 00 00 01 00 00 00` seems correct to me. If I add the `let Inst{11-8} = val` back in I get `db 12 00 00 01 01 00 00` which I don't understand.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72184/new/

https://reviews.llvm.org/D72184