On Fri, Sep 22, 2017 at 9:23 AM, Edward Cree <ec...@solarflare.com> wrote: > On 22/09/17 16:16, Alexei Starovoitov wrote: >> looks like we're converging on >> "be16/be32/be64/le16/le32/le64 #register" for BPF_END. >> I guess it can live with that. I would prefer more C like syntax >> to match the rest, but llvm parsing point is a strong one. > Yep, agreed. I'll post a v2 once we've settled BPF_NEG. >> For BPG_NEG I prefer to do it in C syntax like interpreter does: >> ALU_NEG: >> DST = (u32) -DST; >> ALU64_NEG: >> DST = -DST; >> Yonghong, does it mean that asmparser will equally suffer? > Correction to my earlier statements: verifier will currently disassemble > neg as: > (87) r0 neg 0 > (84) (u32) r0 neg (u32) 0 > because it pretends 'neg' is a compound-assignment operator like +=. > The analogy with be16 and friends would be to use > neg64 r0 > neg32 r0 > whereas the analogy with everything else would be > r0 = -r0 > r0 = (u32) -r0 > as Alexei says. > I'm happy to go with Alexei's version if it doesn't cause problems for llvm.
I got some time to do some prototyping in llvm and it looks like that
I am able to
resolve the issue and we are able to use more C-like syntax. That is:
for bswap:
r1 = (be16) (u16) r1
or
r1 = (be16) r1
or
r1 = be16 r1
for neg:
r0 = -r0
(for 32bit support, llvm may output "w0 = -w0" in the future. But
since it is not
enabled yet, you can continue to output "r0 = (u32) -r0".)
Not sure which syntax is best for bswap. The "r1 = (be16) (u16) r1" is most
explicit in its intention.
Attaching my llvm patch as well and cc'ing Jiong and Jakub so they can see my
implementation and the relative discussion here. (In this patch, I did
not implement
bswap for little endian yet.) Maybe they can provide additional comments.
0001-bpf-add-support-for-neg-insn-and-change-format-of-bs.patch
Description: Binary data
