-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BPF: Change callx insn encoding #81546
Conversation
Currently, the kernel verifier unsupported callx insn used the 32-bit imm field to store the target register. On the other hand, gcc used the dst_reg field to store the target register. The gcc encoding is better. This patch adjusted the coding to be the same as gcc. Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
@llvm/pr-subscribers-mc Author: None (yonghong-song) ChangesCurrently, the kernel verifier unsupported callx insn used the 32-bit imm field to store the target register. On the other hand, gcc used the dst_reg field to store the target register. The gcc encoding is better. This patch adjusted the coding to be the same as gcc. Full diff: https://github.com/llvm/llvm-project/pull/81546.diff 3 Files Affected:
diff --git a/llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp b/llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
index 90697c6645be2f..0d1eef60c3b550 100644
--- a/llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
+++ b/llvm/lib/Target/BPF/AsmParser/BPFAsmParser.cpp
@@ -229,6 +229,7 @@ struct BPFOperand : public MCParsedAsmOperand {
return StringSwitch<bool>(Name.lower())
.Case("if", true)
.Case("call", true)
+ .Case("callx", true)
.Case("goto", true)
.Case("gotol", true)
.Case("*", true)
diff --git a/llvm/lib/Target/BPF/BPFInstrInfo.td b/llvm/lib/Target/BPF/BPFInstrInfo.td
index 7d443a34490146..690d53420718ff 100644
--- a/llvm/lib/Target/BPF/BPFInstrInfo.td
+++ b/llvm/lib/Target/BPF/BPFInstrInfo.td
@@ -622,9 +622,9 @@ class CALLX<string OpcodeStr>
(ins GPR:$BrDst),
!strconcat(OpcodeStr, " $BrDst"),
[]> {
- bits<32> BrDst;
+ bits<4> BrDst;
- let Inst{31-0} = BrDst;
+ let Inst{51-48} = BrDst;
let BPFClass = BPF_JMP;
}
diff --git a/llvm/test/MC/BPF/insn-unit.s b/llvm/test/MC/BPF/insn-unit.s
index 58342cda7cc0ad..224eb7381aa234 100644
--- a/llvm/test/MC/BPF/insn-unit.s
+++ b/llvm/test/MC/BPF/insn-unit.s
@@ -61,6 +61,9 @@
// CHECK-32: c3 92 10 00 00 00 00 00 lock *(u32 *)(r2 + 16) += w9
// CHECK: db a3 e2 ff 00 00 00 00 lock *(u64 *)(r3 - 30) += r10
+ callx r2
+// CHECK: 8d 02 00 00 00 00 00 00 callx r2
+
// ======== BPF_JMP Class ========
if r1 & r2 goto Llabel0 // BPF_JSET | BPF_X
if r1 & 0xffff goto Llabel0 // BPF_JSET | BPF_K
|
cc @jemarch |
Windows test failed. The following are failed tests:
They are all MLIR python tests, and these failures are not really related to this patch. |
@@ -61,6 +61,9 @@ | |||
// CHECK-32: c3 92 10 00 00 00 00 00 lock *(u32 *)(r2 + 16) += w9 | |||
// CHECK: db a3 e2 ff 00 00 00 00 lock *(u64 *)(r3 - 30) += r10 | |||
|
|||
callx r2 | |||
// CHECK: 8d 02 00 00 00 00 00 00 callx r2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am waiting for a compile to finish, but I wanted to say that it looks right to me. The following is a test case from binutils:
28: 8d 06 00 00 00 00 00 00 callr %r6
Note: The mnemonic will change.
Otherwise, everything looks good! Thanks again.
Currently, the kernel verifier unsupported callx insn used the 32-bit imm field to store the target register. On the other hand, gcc used the dst_reg field to store the target register. The gcc encoding is better. This patch adjusted the coding to be the same as gcc.