[llvm] [BPF] Remove 'may_goto 0' instructions (PR #123482)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 24 22:43:27 PST 2025
https://github.com/yonghong-song updated https://github.com/llvm/llvm-project/pull/123482
>From 93019a1f12a6a75d6a1316b0b29670430ead5e95 Mon Sep 17 00:00:00 2001
From: Yonghong Song <yonghong.song at linux.dev>
Date: Mon, 13 Jan 2025 13:31:24 -0800
Subject: [PATCH 1/3] [BPF] Remove 'may_goto 0' instructions
Emil Tsalapatis from Meta reported such a case where 'may_goto 0' insn is
generated by clang compiler. But 'may_goto 0' insn is actually a no-op so
it makes sense to remove that in llvm. The patch is also able to handle
the following code pattern
...
may_goto 2
may_goto 1
may_goto 0
...
where three may_goto insns can all be removed.
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 62 +++++++++++++++++++++++++++
llvm/test/CodeGen/BPF/may_goto.ll | 27 ++++++++++++
2 files changed, 89 insertions(+)
create mode 100644 llvm/test/CodeGen/BPF/may_goto.ll
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index 2b17f2aaefe2bf..eda7a0f0acd363 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -24,6 +24,7 @@
#include "BPFInstrInfo.h"
#include "BPFTargetMachine.h"
#include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/CodeGen/LivePhysRegs.h"
#include "llvm/CodeGen/MachineFrameInfo.h"
#include "llvm/CodeGen/MachineFunctionPass.h"
@@ -322,6 +323,7 @@ struct BPFMIPreEmitPeephole : public MachineFunctionPass {
bool eliminateRedundantMov();
bool adjustBranch();
bool insertMissingCallerSavedSpills();
+ bool removeMayGotoZero();
public:
@@ -337,6 +339,7 @@ struct BPFMIPreEmitPeephole : public MachineFunctionPass {
if (SupportGotol)
Changed = adjustBranch() || Changed;
Changed |= insertMissingCallerSavedSpills();
+ Changed |= removeMayGotoZero();
return Changed;
}
};
@@ -682,6 +685,65 @@ bool BPFMIPreEmitPeephole::insertMissingCallerSavedSpills() {
return Changed;
}
+bool BPFMIPreEmitPeephole::removeMayGotoZero() {
+ bool Changed = false;
+ MachineBasicBlock *Prev_MBB, *Curr_MBB = nullptr;
+ MachineBasicBlock *ToErase = nullptr;
+
+ for (MachineBasicBlock &MBB : reverse(*MF)) {
+ if (ToErase) {
+ ToErase->eraseFromParent();
+ ToErase = nullptr;
+ }
+
+ Prev_MBB = Curr_MBB;
+ Curr_MBB = &MBB;
+ if (Prev_MBB == nullptr)
+ continue;
+
+ MachineInstr &MI = MBB.back();
+ if (!MI.isInlineAsm())
+ continue;
+
+ const char *AsmStr = MI.getOperand(0).getSymbolName();
+ SmallVector<StringRef, 4> AsmPieces;
+ SplitString(AsmStr, AsmPieces, ";\n");
+
+ // Do not support multiple insns in one inline asm.
+ if (AsmPieces.size() != 1)
+ continue;
+
+ // The asm insn must be a may_goto insn.
+ SmallVector<StringRef, 4> AsmOpPieces;
+ SplitString(AsmPieces[0], AsmOpPieces, " ");
+ if (AsmOpPieces[0] != "may_goto")
+ continue;
+
+ // Get the may_goto branch target.
+ MachineOperand &MO = MI.getOperand(InlineAsm::MIOp_FirstOperand + 1);
+ if (MO.getMBB() != Prev_MBB)
+ continue;
+
+ Changed = true;
+ if (MBB.begin() == MI) {
+ // Single 'may_goto' insn in the same basic block.
+ ToErase = Curr_MBB;
+ Curr_MBB->removeSuccessor(Prev_MBB);
+ for (MachineBasicBlock *Pred : Curr_MBB->predecessors())
+ Pred->replaceSuccessor(Curr_MBB, Prev_MBB);
+ Curr_MBB = Prev_MBB;
+ } else {
+ // Remove 'may_goto' insn.
+ MI.eraseFromParent();
+ }
+ }
+
+ if (ToErase)
+ ToErase->eraseFromParent();
+
+ return Changed;
+}
+
} // end default namespace
INITIALIZE_PASS(BPFMIPreEmitPeephole, "bpf-mi-pemit-peephole",
diff --git a/llvm/test/CodeGen/BPF/may_goto.ll b/llvm/test/CodeGen/BPF/may_goto.ll
new file mode 100644
index 00000000000000..a47716509c7aba
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/may_goto.ll
@@ -0,0 +1,27 @@
+; RUN: llc -mtriple=bpfel -mcpu=v3 -filetype=obj -o - %s | llvm-objdump --no-show-raw-insn -d - | FileCheck %s
+
+ at j = dso_local local_unnamed_addr global i32 0, align 4
+
+define dso_local noundef i32 @foo() local_unnamed_addr {
+entry:
+ callbr void asm sideeffect "may_goto ${0:l}", "!i"()
+ to label %for.body [label %for.cond.cleanup]
+
+for.cond.cleanup: ; preds = %for.body.2, %for.body.2, %for.body.1, %for.body, %entry
+ ret i32 0
+
+for.body: ; preds = %entry
+ callbr void asm sideeffect "may_goto ${0:l}", "!i"()
+ to label %for.body.1 [label %for.cond.cleanup]
+
+for.body.1: ; preds = %for.body
+ callbr void asm sideeffect "may_goto ${0:l}", "!i"()
+ to label %for.body.2 [label %for.cond.cleanup]
+
+for.body.2: ; preds = %for.body.1
+ callbr void asm sideeffect "may_goto ${0:l}", "!i"()
+ to label %for.cond.cleanup [label %for.cond.cleanup]
+}
+
+; CHECK: 0: w0 = 0x0
+; CHECK-NEXT: 1: exit
>From af8076c9e64141cf2b3cca8a28bd15eaf1944d4d Mon Sep 17 00:00:00 2001
From: Yonghong Song <yonghong.song at linux.dev>
Date: Fri, 24 Jan 2025 07:54:34 -0800
Subject: [PATCH 2/3] Check INLINEASM_BR instead of INLINEASM
Fix an issue where INLINEASM_BR asm opcode should be checked instead
of generic INLINEASM since later on retrieving target basic block
needs INLINEASM_BR.
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index eda7a0f0acd363..3162027805a65e 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -688,21 +688,15 @@ bool BPFMIPreEmitPeephole::insertMissingCallerSavedSpills() {
bool BPFMIPreEmitPeephole::removeMayGotoZero() {
bool Changed = false;
MachineBasicBlock *Prev_MBB, *Curr_MBB = nullptr;
- MachineBasicBlock *ToErase = nullptr;
-
- for (MachineBasicBlock &MBB : reverse(*MF)) {
- if (ToErase) {
- ToErase->eraseFromParent();
- ToErase = nullptr;
- }
+ for (MachineBasicBlock &MBB : make_early_inc_range(reverse(*MF))) {
Prev_MBB = Curr_MBB;
Curr_MBB = &MBB;
if (Prev_MBB == nullptr)
continue;
MachineInstr &MI = MBB.back();
- if (!MI.isInlineAsm())
+ if (MI.getOpcode() != TargetOpcode::INLINEASM_BR)
continue;
const char *AsmStr = MI.getOperand(0).getSymbolName();
@@ -727,10 +721,10 @@ bool BPFMIPreEmitPeephole::removeMayGotoZero() {
Changed = true;
if (MBB.begin() == MI) {
// Single 'may_goto' insn in the same basic block.
- ToErase = Curr_MBB;
Curr_MBB->removeSuccessor(Prev_MBB);
for (MachineBasicBlock *Pred : Curr_MBB->predecessors())
Pred->replaceSuccessor(Curr_MBB, Prev_MBB);
+ Curr_MBB->eraseFromParent();
Curr_MBB = Prev_MBB;
} else {
// Remove 'may_goto' insn.
@@ -738,9 +732,6 @@ bool BPFMIPreEmitPeephole::removeMayGotoZero() {
}
}
- if (ToErase)
- ToErase->eraseFromParent();
-
return Changed;
}
>From 914ca5ed5aabd85597eda95362791f67fc6108ac Mon Sep 17 00:00:00 2001
From: Yonghong Song <yonghong.song at linux.dev>
Date: Fri, 24 Jan 2025 21:25:13 -0800
Subject: [PATCH 3/3] Ensure correct may_goto asm format
Check AsmOpPieces[1] to ensure may_goto asm format correct.
---
llvm/lib/Target/BPF/BPFMIPeephole.cpp | 5 +++-
.../BPF/{may_goto.ll => may_goto_1.ll} | 0
llvm/test/CodeGen/BPF/may_goto_2.ll | 27 +++++++++++++++++++
3 files changed, 31 insertions(+), 1 deletion(-)
rename llvm/test/CodeGen/BPF/{may_goto.ll => may_goto_1.ll} (100%)
create mode 100644 llvm/test/CodeGen/BPF/may_goto_2.ll
diff --git a/llvm/lib/Target/BPF/BPFMIPeephole.cpp b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
index 3162027805a65e..5f1891d3fce2f2 100644
--- a/llvm/lib/Target/BPF/BPFMIPeephole.cpp
+++ b/llvm/lib/Target/BPF/BPFMIPeephole.cpp
@@ -710,7 +710,10 @@ bool BPFMIPreEmitPeephole::removeMayGotoZero() {
// The asm insn must be a may_goto insn.
SmallVector<StringRef, 4> AsmOpPieces;
SplitString(AsmPieces[0], AsmOpPieces, " ");
- if (AsmOpPieces[0] != "may_goto")
+ if (AsmOpPieces.size() != 2 || AsmOpPieces[0] != "may_goto")
+ continue;
+ // Enforce the format of 'may_goto <label>'.
+ if (AsmOpPieces[1] != "${0:l}" && AsmOpPieces[1] != "$0")
continue;
// Get the may_goto branch target.
diff --git a/llvm/test/CodeGen/BPF/may_goto.ll b/llvm/test/CodeGen/BPF/may_goto_1.ll
similarity index 100%
rename from llvm/test/CodeGen/BPF/may_goto.ll
rename to llvm/test/CodeGen/BPF/may_goto_1.ll
diff --git a/llvm/test/CodeGen/BPF/may_goto_2.ll b/llvm/test/CodeGen/BPF/may_goto_2.ll
new file mode 100644
index 00000000000000..8f6fea5913d2ab
--- /dev/null
+++ b/llvm/test/CodeGen/BPF/may_goto_2.ll
@@ -0,0 +1,27 @@
+; RUN: llc -mtriple=bpfel -mcpu=v3 -filetype=obj -o - %s | llvm-objdump --no-show-raw-insn -d - | FileCheck %s
+
+ at j = dso_local local_unnamed_addr global i32 0, align 4
+
+define dso_local noundef i32 @foo() local_unnamed_addr {
+entry:
+ callbr void asm sideeffect "may_goto $0", "!i"()
+ to label %for.body [label %for.cond.cleanup]
+
+for.cond.cleanup: ; preds = %for.body.2, %for.body.2, %for.body.1, %for.body, %entry
+ ret i32 0
+
+for.body: ; preds = %entry
+ callbr void asm sideeffect "may_goto $0", "!i"()
+ to label %for.body.1 [label %for.cond.cleanup]
+
+for.body.1: ; preds = %for.body
+ callbr void asm sideeffect "may_goto $0", "!i"()
+ to label %for.body.2 [label %for.cond.cleanup]
+
+for.body.2: ; preds = %for.body.1
+ callbr void asm sideeffect "may_goto $0", "!i"()
+ to label %for.cond.cleanup [label %for.cond.cleanup]
+}
+
+; CHECK: 0: w0 = 0x0
+; CHECK-NEXT: 1: exit
More information about the llvm-commits
mailing list