[clang] [llvm] [Clang][BPF] Allow sign extension for call parameters with int types (PR #84874)

2024-03-14 Thread Pu Lehui via cfe-commits

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)

2024-03-14 Thread Pu Lehui via cfe-commits

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)

2024-03-14 Thread via cfe-commits

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)

2024-03-13 Thread via cfe-commits

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)

2024-03-13 Thread via cfe-commits

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)

2024-03-13 Thread via cfe-commits

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