[BPF] Define set of BPF libcalls

[LucasSte]

Problem

Following the comment in #168442 (review), we can define which libcalls are available in BPF, so that i128 arithmetics and i64 arithmetics with overflow detection can correctly work in BPF.

Solution

Exclude from the default set all i128 related arithmetic operations.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Lucas Ste (LucasSte)

Changes

Problem

Following the comment in #168442 (review), we can define which libcalls are available in BPF, so that i128 arithmetics and i64 arithmetics with overflow detection can correctly work in BPF.

Solution

Exclude from the default set all i128 related arithmetic operations.

---

Full diff: https://github.com/llvm/llvm-project/pull/169537.diff

3 Files Affected:

• (modified) llvm/include/llvm/IR/RuntimeLibcalls.td (+17)
• (removed) llvm/test/CodeGen/BPF/builtin_calls.ll (-39)
• (added) llvm/test/CodeGen/BPF/i128_math.ll (+23)

diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.td b/llvm/include/llvm/IR/RuntimeLibcalls.td
index 11e6127e0741d..d0e2eafdd0538 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.td
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.td
@@ -2808,6 +2808,23 @@ def WasmSystemLibrary
            emscripten_return_address,
            __stack_chk_fail, __stack_chk_guard)>;
 
+//===----------------------------------------------------------------------===//
+// BPF Runtime Libcalls
+//===----------------------------------------------------------------------===//
+
+def isBPF : RuntimeLibcallPredicate<"TT.isBPF()">;
+
+def BPFSystemLibrary
+    : SystemRuntimeLibrary<isBPF,
+      (add (sub DefaultRuntimeLibcallImpls,
+      // i128 operations should be lowered by LLVM for BPF
+       __divti3,
+       __udivti3,
+       __modti3,
+       __umodti3,
+       __clzti2
+    ))>;
+
 //===----------------------------------------------------------------------===//
 // Legacy Default Runtime Libcalls
 //===----------------------------------------------------------------------===//
diff --git a/llvm/test/CodeGen/BPF/builtin_calls.ll b/llvm/test/CodeGen/BPF/builtin_calls.ll
deleted file mode 100644
index 18199eba7222a..0000000000000
--- a/llvm/test/CodeGen/BPF/builtin_calls.ll
+++ /dev/null
@@ -1,39 +0,0 @@
-; RUN: llc -march=bpfel -mattr=+allow-builtin-calls < %s | FileCheck %s
-;
-; C code for this test case:
-;
-; long func(long a, long b) {
-;     long x;
-;     return __builtin_mul_overflow(a, b, &x);
-; }
-
-
-declare { i64, i1 } @llvm.smul.with.overflow.i64(i64, i64)
-
-define noundef range(i64 0, 2) i64 @func(i64 noundef %a, i64 noundef %b) local_unnamed_addr {
-entry:
-  %0 = tail call { i64, i1 } @llvm.smul.with.overflow.i64(i64 %a, i64 %b)
-  %1 = extractvalue { i64, i1 } %0, 1
-  %conv = zext i1 %1 to i64
-  ret i64 %conv
-}
-
-; CHECK-LABEL: func
-; CHECK: r4 = r2
-; CHECK: r2 = r1
-; CHECK: r3 = r2
-; CHECK: r3 s>>= 63
-; CHECK: r5 = r4
-; CHECK: r5 s>>= 63
-; CHECK: r1 = r10
-; CHECK: r1 += -16
-; CHECK: call __multi3
-; CHECK: r1 = *(u64 *)(r10 - 16)
-; CHECK: r1 s>>= 63
-; CHECK: w0 = 1
-; CHECK: r2 = *(u64 *)(r10 - 8)
-; CHECK: if r2 != r1 goto LBB0_2
-; CHECK:  # %bb.1:                                # %entry
-; CHECK: w0 = 0
-; CHECK:  LBB0_2:                                 # %entry
-; CHECK: exit
\ No newline at end of file
diff --git a/llvm/test/CodeGen/BPF/i128_math.ll b/llvm/test/CodeGen/BPF/i128_math.ll
new file mode 100644
index 0000000000000..6fbbdfe4662c3
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/i128_math.ll
@@ -0,0 +1,23 @@
+; RUN: llc -march=bpfel < %s | FileCheck %s
+;
+; C code for this test case:
+;
+; long func(long a, long b) {
+;     long x;
+;     return __builtin_mul_overflow(a, b, &x);
+; }
+
+
+declare { i64, i1 } @llvm.smul.with.overflow.i64(i64, i64)
+
+define noundef range(i64 0, 2) i64 @func(i64 noundef %a, i64 noundef %b) local_unnamed_addr {
+entry:
+  %0 = tail call { i64, i1 } @llvm.smul.with.overflow.i64(i64 %a, i64 %b)
+  %1 = extractvalue { i64, i1 } %0, 1
+  %conv = zext i1 %1 to i64
+  ret i64 %conv
+}
+
+; CHECK-LABEL: func
+; CHECK-NOT: call __multi3
+; CHECK: exit

[LucasSte]

If we can merge this PR, I can remove the sub target feature I created in #168442 or even revert those changes altogether.

cc. @yonghong-song and @arsenm

[LucasSte]

If we can merge this PR, I can remove the sub target feature I created in #168442 or even revert those changes altogether.

cc. @yonghong-song and @arsenm

Sorry for the ping. On a second look, I need to rethink this change a little bit. While it was suggested that we tell which lib calls are available for BPF, if I simply say __udivti3 is not available, I get a codegen error unsupported library call operation.

In the meantime, the best solution for my use case is still allowing BPF to emit lib calls under a sub target feature while linking them agains Rust's compiler-builtins library.

[LucasSte]

Sorry for the ping. On a second look, I need to rethink this change a little bit. While it was suggested that we tell which lib calls are available for BPF, if I simply say __udivti3 is not available, I get a codegen error unsupported library call operation.

i128 will always expand in LLVM for BPF, so the DAG legalizer will call ExpandIntRes_UDIV for udiv, which will always invoke the lib call.

I might be missing something, but I believe the change in this PR is not possible.

[arsenm]

Since the error seems to be emit an error on any external call, it might be more appropriate to just remove all calls (i.e., copy the AMDGPU implementation). This would also fix memcpy and other cases: https://godbolt.org/z/WvnEMMx1c

I'm also surprised that your previous patch seemed to work by relying on compiler-rt, if compiler-rt isn't built and linked by the toolchain? Ideally the set of available calls would be a conscious ABI definition

[arsenm]

i128 will always expand in LLVM for BPF, so the DAG legalizer will call ExpandIntRes_UDIV for udiv, which will always invoke the lib call.

I might be missing something, but I believe the change in this PR is not possible.

For i128 division you need to add createExpandLargeDivRemPass to the pass pipeline; it's broken that this isn't in the default codegen pipeline.

Rust's compiler-builtins library.

This sounds like it should have a separate triple, and designed ABI?