jackmanb added inline comments.
================
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;
----------------
jackmanb wrote:
> jackmanb wrote:
> > 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.
> Ah wait, I guess this is adding a 3rd operand register? In my example it's
> unclear because it is R1 which is also the dst operand. I was envisaging we
> just have semantics like `src = atomic_fetch_add(dst+off, src)` but you are
> instead proposing `dst = atomic_fetch_add(dst+off, val)`?
Sorry I mean `dst = atomic_fetch_add(src+off, val)`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72184/new/
https://reviews.llvm.org/D72184
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
