[llvm] [BPF] fix sub-register handling for bpf_fastcall (PR #110618)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 30 21:42:26 PDT 2024
https://github.com/eddyz87 updated https://github.com/llvm/llvm-project/pull/110618
>From 407143cbe24d9d5b2ae8f8c633cd7482749bacda Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87 at gmail.com>
Date: Mon, 30 Sep 2024 18:27:20 -0700
Subject: [PATCH 1/4] [BPF] fix sub-register handling for bpf_fastcall
bpf_fastcall induced spill/fill pairs should be generated for
sub-register as well as for sub-registers. At the moment this is not
the case, e.g.:
$ cat t.c
extern int foo(void) __attribute__((bpf_fastcall));
int bar(int a) {
foo();
return a;
}
$ clang --target=bpf -mcpu=v3 -O2 -S t.c -o -
...
call foo
w0 = w1
exit
Modify BPFMIPeephole.cpp:collectBPFFastCalls() to check sub-registers
liveness and thus produce correct code for example above:
*(u64 *)(r10 - 8) = r1
call foo
r1 = *(u64 *)(r10 - 8)
w0 = w1
exit
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 11 +++++++-
llvm/test/CodeGen/BPF/bpf-fastcall-4.ll | 35 +++++++++++++++++++++++++
llvm/test/CodeGen/BPF/bpf-fastcall-5.ll | 35 +++++++++++++++++++++++++
3 files changed, 80 insertions(+), 1 deletion(-)
create mode 100644 llvm/test/CodeGen/BPF/bpf-fastcall-4.ll
create mode 100644 llvm/test/CodeGen/BPF/bpf-fastcall-5.ll
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index c41eab319dbb9b..a2f18b5c8c355e 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -614,11 +614,20 @@ static void collectBPFFastCalls(const TargetRegisterInfo *TRI,
LiveRegs.init(*TRI);
LiveRegs.addLiveOuts(BB);
Calls.clear();
+
+ auto IsLiveBeforeInsn = [&](MCRegister R, MachineInstr &MI) {
+ return !MI.definesRegister(R, TRI) && LiveRegs.contains(R);
+ };
+
for (MachineInstr &MI : llvm::reverse(BB)) {
if (MI.isCall()) {
unsigned LiveCallerSavedRegs = 0;
for (MCRegister R : CallerSavedRegs) {
- bool DoSpillFill = !MI.definesRegister(R, TRI) && LiveRegs.contains(R);
+ bool DoSpillFill = IsLiveBeforeInsn(R, MI);
+ for (MCSubRegIndexIterator SRI(R, TRI); SRI.isValid(); ++SRI) {
+ MCRegister SR = SRI.getSubReg();
+ DoSpillFill |= IsLiveBeforeInsn(SR, MI);
+ }
if (!DoSpillFill)
continue;
LiveCallerSavedRegs |= 1 << R;
diff --git a/llvm/test/CodeGen/BPF/bpf-fastcall-4.ll b/llvm/test/CodeGen/BPF/bpf-fastcall-4.ll
new file mode 100644
index 00000000000000..4f0c9fc94aa9df
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/bpf-fastcall-4.ll
@@ -0,0 +1,35 @@
+; RUN: llc -O2 --march=bpfel %s -o - | FileCheck %s
+
+; Generated from the following C code:
+;
+; extern int foo(void) __attribute__((bpf_fastcall));
+;
+; int bar(int a) {
+; foo();
+; return a;
+; }
+;
+; Using the following command:
+;
+; clang --target=bpf -emit-llvm -O2 -S -o - t.c
+;
+; (unnecessary attrs removed maually)
+
+; Check that function marked with bpf_fastcall does not clobber W1-W5.
+
+define dso_local i32 @bar(i32 %a) {
+entry:
+ %call = tail call i32 @foo() #0
+ ret i32 %a
+}
+
+; CHECK: # %bb.0:
+; CHECK-NEXT: *(u64 *)(r10 - 8) = r1
+; CHECK-NEXT: call foo
+; CHECK-NEXT: r1 = *(u64 *)(r10 - 8)
+; CHECK-NEXT: w0 = w1
+; CHECK-NEXT: exit
+
+declare dso_local i32 @foo() #0
+
+attributes #0 = { "bpf_fastcall" }
diff --git a/llvm/test/CodeGen/BPF/bpf-fastcall-5.ll b/llvm/test/CodeGen/BPF/bpf-fastcall-5.ll
new file mode 100644
index 00000000000000..608ab4a0bb0cfd
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/bpf-fastcall-5.ll
@@ -0,0 +1,35 @@
+; RUN: llc -O2 --march=bpfel %s -o - | FileCheck %s
+
+; Generated from the following C code:
+;
+; extern int foo(void) __attribute__((bpf_fastcall));
+;
+; int bar(int a, int b, int c, int d, int e) {
+; foo();
+; return e;
+; }
+;
+; Using the following command:
+;
+; clang --target=bpf -emit-llvm -O2 -S -o - t.c
+;
+; (unnecessary attrs removed maually)
+
+; Check that function marked with bpf_fastcall does not clobber W1-W5.
+
+define dso_local i32 @bar(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e) {
+entry:
+ %call = tail call i32 @foo() #0
+ ret i32 %e
+}
+
+; CHECK: # %bb.0:
+; CHECK-NEXT: *(u64 *)(r10 - 8) = r5
+; CHECK-NEXT: call foo
+; CHECK-NEXT: r5 = *(u64 *)(r10 - 8)
+; CHECK-NEXT: w0 = w5
+; CHECK-NEXT: exit
+
+declare dso_local i32 @foo() #0
+
+attributes #0 = { "bpf_fastcall" }
>From f5e413dd78f96ac56ff1f41cbe6c0c50b2bd7f09 Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87 at gmail.com>
Date: Mon, 30 Sep 2024 19:53:16 -0700
Subject: [PATCH 2/4] drop first call to IsLiveBeforeInsn
As pointed by @4ast, there is no need to do two calls to
`IsLiveBeforeInsn`, LivePhysRegs::contains() is documented as follows:
/// ... This also
/// works if only the super register of \p Reg has been defined, because
/// addReg() always adds all sub-registers to the set as well ...
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index a2f18b5c8c355e..007e2da6284807 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -615,18 +615,14 @@ static void collectBPFFastCalls(const TargetRegisterInfo *TRI,
LiveRegs.addLiveOuts(BB);
Calls.clear();
- auto IsLiveBeforeInsn = [&](MCRegister R, MachineInstr &MI) {
- return !MI.definesRegister(R, TRI) && LiveRegs.contains(R);
- };
-
for (MachineInstr &MI : llvm::reverse(BB)) {
if (MI.isCall()) {
unsigned LiveCallerSavedRegs = 0;
for (MCRegister R : CallerSavedRegs) {
- bool DoSpillFill = IsLiveBeforeInsn(R, MI);
+ bool DoSpillFill = false;
for (MCSubRegIndexIterator SRI(R, TRI); SRI.isValid(); ++SRI) {
MCRegister SR = SRI.getSubReg();
- DoSpillFill |= IsLiveBeforeInsn(SR, MI);
+ DoSpillFill |= !MI.definesRegister(SR, TRI) && LiveRegs.contains(SR);
}
if (!DoSpillFill)
continue;
>From 4c79104ec5e3b0ff7de17920e6b137d39729b087 Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87 at gmail.com>
Date: Mon, 30 Sep 2024 19:57:57 -0700
Subject: [PATCH 3/4] Use TargetRegisterInfo::subregs() instead of
MCSubRegIndexIterator
No changes in behaviour, just makes the code more terse.
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index 007e2da6284807..83c98724550971 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -620,10 +620,8 @@ static void collectBPFFastCalls(const TargetRegisterInfo *TRI,
unsigned LiveCallerSavedRegs = 0;
for (MCRegister R : CallerSavedRegs) {
bool DoSpillFill = false;
- for (MCSubRegIndexIterator SRI(R, TRI); SRI.isValid(); ++SRI) {
- MCRegister SR = SRI.getSubReg();
+ for (MCPhysReg SR : TRI->subregs(R))
DoSpillFill |= !MI.definesRegister(SR, TRI) && LiveRegs.contains(SR);
- }
if (!DoSpillFill)
continue;
LiveCallerSavedRegs |= 1 << R;
>From 633a5c0810b5a571b3065695086adc1d6b77ad21 Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87 at gmail.com>
Date: Mon, 30 Sep 2024 21:41:03 -0700
Subject: [PATCH 4/4] whitespace cleanup
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index 83c98724550971..2b17f2aaefe2bf 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -614,7 +614,6 @@ static void collectBPFFastCalls(const TargetRegisterInfo *TRI,
LiveRegs.init(*TRI);
LiveRegs.addLiveOuts(BB);
Calls.clear();
-
for (MachineInstr &MI : llvm::reverse(BB)) {
if (MI.isCall()) {
unsigned LiveCallerSavedRegs = 0;
More information about the llvm-commits
mailing list