[PATCH] [RFC PATCH] BPF backend
Tom Stellard
tom at stellard.net
Wed Dec 3 15:11:14 PST 2014
On Wed, Dec 03, 2014 at 02:50:41PM -0800, Alexei Starovoitov wrote:
> On Wed, Dec 3, 2014 at 1:53 PM, Tom Stellard <thomas.stellard at amd.com> wrote:
> > ================
> > Comment at: configure:5309-5315
> > @@ -5309,7 +5309,7 @@
> >
> > fi
> >
> > -ALL_TARGETS="X86 Sparc PowerPC ARM AArch64 Mips XCore MSP430 CppBackend NVPTX Hexagon SystemZ R600"
> > +ALL_TARGETS="X86 Sparc PowerPC ARM AArch64 Mips XCore MSP430 CppBackend NVPTX Hexagon SystemZ R600 BPF"
> > ALL_TARGETS=$ALL_TARGETS
> >
> >
> > ----------------
> > You need to add this target to the CMake build system too.
>
> ok. will add cmake bits as well.
>
> > ================
> > Comment at: lib/Target/BPF/BPFISelLowering.cpp:434-435
> > @@ +433,4 @@
> > +
> > +static bool NegateCC(SDValue &LHS, SDValue &RHS, ISD::CondCode &CC)
> > +{
> > + switch (CC) {
> > ----------------
> > Coding style: Bracket should be on same line as function.
>
> ohh yes. will do.
>
> > ================
> > Comment at: lib/Target/BPF/BPFISelLowering.cpp:436-456
> > @@ +435,23 @@
> > +{
> > + switch (CC) {
> > + default:
> > + return false;
> > + case ISD::SETULT:
> > + CC = ISD::SETUGT;
> > + std::swap(LHS, RHS);
> > + return true;
> > + case ISD::SETULE:
> > + CC = ISD::SETUGE;
> > + std::swap(LHS, RHS);
> > + return true;
> > + case ISD::SETLT:
> > + CC = ISD::SETGT;
> > + std::swap(LHS, RHS);
> > + return true;
> > + case ISD::SETLE:
> > + CC = ISD::SETGE;
> > + std::swap(LHS, RHS);
> > + return true;
> > + }
> > +}
> > +
> > ----------------
> > You may be able to simplify this using ISD::getSetCCSwappedOperands().
>
> yes. good point. will do
>
> > ================
> > Comment at: lib/Target/BPF/BPFInstrInfo.td:96-99
> > @@ +95,6 @@
> > +// jump instructions
> > +class JMP_RR<bits<4> br_op, string asmstr, PatLeaf Cond>
> > + : InstBPF<(outs), (ins GPR:$rA, GPR:$rX, brtarget:$dst),
> > + !strconcat(asmstr, "\t$rA, $rX goto $dst"),
> > + [(BPFbrcc (i64 GPR:$rA), (i64 GPR:$rX), Cond, bb:$dst)]> {
> > + bits<4> op;
> > ----------------
> > You don't need to specify register classes any more, so you can rewrite this pattern as:
> >
> > (BPFbrcc i64:$rA, i64:$rX, Cond, bb:$dst)]
> >
> > Some comment for most other patterns in this file.
>
> ok. makes sense. that will definitely make it less verbose.
>
> > ================
> > Comment at: lib/Target/BPF/BPFInstrInfo.td:501-504
> > @@ +500,6 @@
> > +
> > +class LOAD_IND<bits<2> sizeOp, string asmstr, Intrinsic opNode>
> > + : InstBPF<(outs), (ins GPR:$skb, GPR:$val),
> > + !strconcat(asmstr, "\tr0, $skb.data + $val"),
> > + [(set R0, (opNode GPR:$skb, GPR:$val))]> {
> > + bits<3> mode;
> > ----------------
> > I did not realize this was possible. Does the instruction selector add R0 as an implicit def?
>
> yep. it looks like this:
> LD_ABS_H %R6, 12, %R0<imp-def>, %R1<imp-def,dead>, %R2<imp-def,dead>,
> %R3<imp-def,dead>, %R4<imp-def,dead>, %R5<imp-def,dead>, %R6<imp-use>
> %vreg57<def> = COPY %R0; GPR:%vreg57
>
> I'm not sure it's documented, since I don't see other backends use it.
> Should I change to something else?
> What's a recommended approach?
>
It's fine as is, I just wanted to know how it worked so I could use it too.
> > ================
> > Comment at: lib/Target/BPF/BPFRegisterInfo.td:7-8
> > @@ +6,4 @@
> > +
> > +class BPFReg<string n> : Register<n> {
> > + field bits<4> Num;
> > + let Namespace = "BPF";
> > ----------------
> > I'm not sure what you are using Num here for, but Register has a HWEncoding field which is accessible from the CodeEmitter if you need it.
>
> left over of copy-paste. will remove.
>
> > ================
> > Comment at: lib/Target/BPF/MCTargetDesc/BPFBaseInfo.h:14-30
> > @@ +13,19 @@
> > +
> > +static inline unsigned getBPFRegisterNumbering(unsigned Reg) {
> > + switch(Reg) {
> > + case BPF::R0 : return 0;
> > + case BPF::R1 : return 1;
> > + case BPF::R2 : return 2;
> > + case BPF::R3 : return 3;
> > + case BPF::R4 : return 4;
> > + case BPF::R5 : return 5;
> > + case BPF::R6 : return 6;
> > + case BPF::R7 : return 7;
> > + case BPF::R8 : return 8;
> > + case BPF::R9 : return 9;
> > + case BPF::R10 : return 10;
> > + case BPF::R11 : return 11;
> > + default: llvm_unreachable("Unknown register number!");
> > + }
> > +}
> > +
> > ----------------
> > See my previous comment about the HWEncoding field for registers, you can have TableGen generate this for you.
>
> ok. will do. When I started that's how other backends were doing it.
>
> > ================
> > Comment at: lib/Target/BPF/MCTargetDesc/BPFMCCodeEmitter.cpp:127-147
> > @@ +126,23 @@
> > +
> > + if (Opcode == BPF::LD_imm64) {
> > + uint64_t Value = getBinaryCodeForInstr(MI, Fixups, STI);
> > + EmitByte(Value >> 56, CurByte, OS);
> > + EmitByte(((Value >> 48) & 0xff), CurByte, OS);
> > + EmitLEConstant(0, 2, CurByte, OS);
> > + EmitLEConstant(Value & 0xffffFFFF, 4, CurByte, OS);
> > +
> > + const MCOperand &MO = MI.getOperand(1);
> > + uint64_t Imm = MO.isImm() ? MO.getImm() : 0;
> > + EmitByte(0, CurByte, OS);
> > + EmitByte(0, CurByte, OS);
> > + EmitLEConstant(0, 2, CurByte, OS);
> > + EmitLEConstant(Imm >> 32, 4, CurByte, OS);
> > + } else {
> > + // Get instruction encoding and emit it
> > + uint64_t Value = getBinaryCodeForInstr(MI, Fixups, STI);
> > + EmitByte(Value >> 56, CurByte, OS);
> > + EmitByte((Value >> 48) & 0xff, CurByte, OS);
> > + EmitLEConstant((Value >> 32) & 0xffff, 2, CurByte, OS);
> > + EmitLEConstant(Value & 0xffffFFFF, 4, CurByte, OS);
> > + }
> > +}
> > ----------------
> > Why can't you just emit the value returned by getBinaryCodeForInstr() directly without re-ordering the bytes?
>
> It's easier to encode all bits of insn in BPFInstrInfo.td this way.
> Right now it emits right thing for little endian only, but encoding should
> be big if underlying cpu is big-endian. So more 'if' conditions will
> be added here in the future.
>
There might be a way to have LLVM emit with the correct endianness for you.
AArch64 has big and little endian targets, might want to see what it does.
-Tom
> Thanks a lot for review!
>
> > http://reviews.llvm.org/D6494
> >
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list