[BPF] Define empty set of BPF libcalls

[LucasSte]

Problem

BPF does not offer native support for any libcall, but it uses the default legacy set of lib calls in LLVM. That is inconvenient because we can't use Rust's u128 and i128 types with BPF without hitting and invalid libcall error.

This PR is a suggestion in #168442 (review), so that I can revert the original PR and implement the right solution.

Solution

Removing lib calls for BPF allows LLVM to lower i128 operations without runtime libraries.

[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?

[LucasSte]

Hey @arsenm, thanks a lot for your feedback. I'll implement your suggestions for now and revert #168442.

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

I have tested that locally and found it to work. I'll create a separate PR for that.

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

I believe that is starting to make sense, yes. I'll first fix the i128 operations in BPF, then I'll think about that.

[github-actions]

🐧 Linux x64 Test Results

• 186631 tests passed
• 4884 tests skipped