[llvm] BPF: Generate locked insn for __sync_fetch_and_add() with cpu v1/v2 (PR #106494)

via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 12 06:15:51 PDT 2024


th0rex wrote:

Hi, this commit broke some of our code, I am not sure whether this is intentional or not. It seems to me that anything using the return value of e.g. `__sync_fetch_and_add` doesn't work anymore with `-mcpu=v2` (nor with `-mcpu=v1`).

To reproduce:

On the parent of this commit (57fe53cae40351ebd079a9a0105addf4ad2e97dd) the following test is working:

```
~/llvm-project ((57fe53ca))> git status
HEAD detached at 57fe53cae403
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   llvm/test/CodeGen/BPF/atomics.ll

no changes added to commit (use "git add" and/or "git commit -a")

~/llvm-project ((57fe53ca))> git diff
diff --git a/llvm/test/CodeGen/BPF/atomics.ll b/llvm/test/CodeGen/BPF/atomics.ll
index 0c16c49f2a87..5e71dcef85a2 100644
--- a/llvm/test/CodeGen/BPF/atomics.ll
+++ b/llvm/test/CodeGen/BPF/atomics.ll
@@ -19,3 +19,12 @@ entry:
   atomicrmw add ptr %p, i64 %v seq_cst
   ret void
 }
+
+; CHECK-LABEL: test_load_add_ret_64
+; CHECK: r2 = atomic_fetch_add((u64 *)(r1 + 0), r2)
+; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x01,0x00,0x00,0x00]
+define i64 @test_load_add_ret_64(ptr %p, i64 zeroext %v) {
+entry:
+  %0 = atomicrmw add ptr %p, i64 %v seq_cst
+  ret i64 %0
+}

~/llvm-project ((57fe53ca))> ./build/bin/llc -march=bpfel -mcpu=v2 llvm/test/CodeGen/BPF/atomics.ll
~/llvm-project ((57fe53ca))> # runs successfully
```

With this commit (06c531e808ceeafdf996867a2e8e66960ae4774e), the same test fails:

```
~/llvm-project ((06c531e8))> git status
HEAD detached at 06c531e808ce
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   llvm/test/CodeGen/BPF/atomics.ll

no changes added to commit (use "git add" and/or "git commit -a")

~/llvm-project ((06c531e8))> git diff
diff --git a/llvm/test/CodeGen/BPF/atomics.ll b/llvm/test/CodeGen/BPF/atomics.ll
index c17b94af5f7b..fbccfe54135e 100644
--- a/llvm/test/CodeGen/BPF/atomics.ll
+++ b/llvm/test/CodeGen/BPF/atomics.ll
@@ -22,3 +22,12 @@ entry:
   atomicrmw add ptr %p, i64 %v seq_cst
   ret void
 }
+
+; CHECK-LABEL: test_load_add_ret_64
+; CHECK: r2 = atomic_fetch_add((u64 *)(r1 + 0), r2)
+; CHECK: encoding: [0xdb,0x21,0x00,0x00,0x01,0x00,0x00,0x00]
+define i64 @test_load_add_ret_64(ptr %p, i64 zeroext %v) {
+entry:
+  %0 = atomicrmw add ptr %p, i64 %v seq_cst
+  ret i64 %0
+}

~/llvm-project ((06c531e8))> ./build/bin/llc -march=bpfel -mcpu=v2 llvm/test/CodeGen/BPF/atomics.ll
error: <unknown>:0:0: in function test_load_add_ret_64 i64 (ptr, i64): Invalid usage of the XADD return value
```

Like I said, I'm not sure if this is intentional or not - we just switched to inline assembly and call `atomic_fetch_add` directly rather than relying on `__sync_fetch_and_add` which still works as expected, but I wanted to bring this to your attention in case this is not intentional.

https://github.com/llvm/llvm-project/pull/106494


More information about the llvm-commits mailing list