[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
pulehui wrote: > @pulehui I will not merge this patch now since it does not really solve the > whole riscv issue. As @4ast commented above, the verifier made btf_func_model > to jit. > [snip] > So you will know whether it is a struct or signed or not. If you feel you > need explicit UNSIGNED flag, you can add it in btf as well. > > Let us try to resolve the issue in riscv backend. Thanks! Thanks Yonghong and Alexei, will try to resolve this incompatibility between bpf abi and riscv abi. https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
pulehui wrote: > The `r1` would be zero extended, while to be ABI conformant it has to be sign > extended, as for RV64 the 31-st bit should be the same as upper 32 bits. The > fact that decision to zero or sign extend the argument depends on the > architecture means that this extension has to be done at JIT time (meaning > that BTF is mandatory) or be a part of kfunc. Thanks for the clarification. riscv64 will do sign-extension both for int and uint. For uint, zero-extension is required, but after my investigation, it will be handled in the callee. https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
yonghong-song wrote: @pulehui I will not merge this patch now since it does not really solve the whole riscv issue. As @4ast commented above, the verifier made btf_func_model to jit. ``` struct btf_func_model { u8 ret_size; u8 ret_flags; u8 nr_args; u8 arg_size[MAX_BPF_FUNC_ARGS]; u8 arg_flags[MAX_BPF_FUNC_ARGS]; }; ``` For example, for each argument, you will know its size and flags has some information as well.The flags has ``` static u8 __get_type_fmodel_flags(const struct btf_type *t) { u8 flags = 0; if (__btf_type_is_struct(t)) flags |= BTF_FMODEL_STRUCT_ARG; if (btf_type_is_signed_int(t)) flags |= BTF_FMODEL_SIGNED_ARG; return flags; } ``` So you will know whether it is a struct or signed or not. If you feel you need explicit UNSIGNED flag, you can add it in btf as well. Let us try to resolve the issue in riscv backend. Thanks! https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
4ast wrote: > Just uploaded a new revision which targets 'Int' type only, i.e, v1. Looks like it's a bug in risc-v JIT. We never promised that kfunc call from bpf calling convention into native cpu calling convention will be free on all architectures. That's why we have btf_func_model and JITs suppose to use it when calling conventions don't match. It seems risc-v JIT needs to emit sign extension when 'int' argument is passed into kfunc. Simple stuff, comparing to what we do in 32-bit x86 JIT where 64-bit bpf program can call into 32-bit x86 kfunc. https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
yonghong-song wrote: @pulehui On top of this patch, the following is a hack to emit 'sign-extension' for uint types (unsigned char/short not affected). The llvm hack: ``` diff --git a/llvm/lib/Target/BPF/BPFISelLowering.cpp b/llvm/lib/Target/BPF/BPFISelLowering.cpp index b8ca8ec98c06..e4ab04c99c60 100644 --- a/llvm/lib/Target/BPF/BPFISelLowering.cpp +++ b/llvm/lib/Target/BPF/BPFISelLowering.cpp @@ -37,6 +37,10 @@ static cl::opt BPFExpandMemcpyInOrder("bpf-expand-memcpy-in-order", cl::Hidden, cl::init(false), cl::desc("Expand memcpy into load/store pairs in order")); +static cl::opt BPFForRISCV("bpf-for-riscv", + cl::Hidden, cl::init(false), + cl::desc("BPF prog intended to run on riscv arch")); + static void fail(const SDLoc , SelectionDAG , const Twine , SDValue Val = {}) { std::string Str; @@ -239,6 +243,12 @@ bool BPFTargetLowering::isZExtFree(SDValue Val, EVT VT2) const { return TargetLoweringBase::isZExtFree(Val, VT2); } +bool BPFTargetLowering::isSExtCheaperThanZExt(EVT SrcVT, EVT DstVT) const { + if (BPFForRISCV) +return SrcVT == MVT::i32 && DstVT == MVT::i64; + return TargetLoweringBase::isSExtCheaperThanZExt(SrcVT, DstVT); +} + BPFTargetLowering::ConstraintType BPFTargetLowering::getConstraintType(StringRef Constraint) const { if (Constraint.size() == 1) { diff --git a/llvm/lib/Target/BPF/BPFISelLowering.h b/llvm/lib/Target/BPF/BPFISelLowering.h index 42707949e864..e157a402b6e0 100644 --- a/llvm/lib/Target/BPF/BPFISelLowering.h +++ b/llvm/lib/Target/BPF/BPFISelLowering.h @@ -152,6 +152,7 @@ private: bool isZExtFree(Type *Ty1, Type *Ty2) const override; bool isZExtFree(EVT VT1, EVT VT2) const override; bool isZExtFree(SDValue Val, EVT VT2) const override; + bool isSExtCheaperThanZExt(EVT SrcVT, EVT DstVT) const override; unsigned EmitSubregExt(MachineInstr , MachineBasicBlock *BB, unsigned Reg, bool isSigned) const; ``` The following is a simple example, ``` $ cat t.c void bar(unsigned int, unsigned int); void foo() { bar(5, -5); } $ clang --target=bpf -O2 -c t.c && llvm-objdump -d t.o t.o:file format elf64-bpf Disassembly of section .text: : 0: b7 01 00 00 05 00 00 00 r1 = 0x5 1: 18 02 00 00 fb ff ff ff 00 00 00 00 00 00 00 00 r2 = 0xfffb ll 3: 85 10 00 00 ff ff ff ff call -0x1 4: 95 00 00 00 00 00 00 00 exit $ clang --target=bpf -O2 -c t.c -mllvm -bpf-for-riscv && llvm-objdump -d t.o t.o:file format elf64-bpf Disassembly of section .text: : 0: b7 01 00 00 05 00 00 00 r1 = 0x5 1: b7 02 00 00 fb ff ff ff r2 = -0x5 2: 85 10 00 00 ff ff ff ff call -0x1 3: 95 00 00 00 00 00 00 00 exit $ ``` But I really do not like to generate BPF codes for different architectures. Maybe riscv can propose some code patterns where bpf backend can generate generic code (for all arches) and those codes can be easier for riscv jit to do proper sign extension? https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)
https://github.com/yonghong-song edited https://github.com/llvm/llvm-project/pull/84874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits