[PATCH] [v7] BPF backend

[Alexei Starovoitov]

Thanks for the review!
Will wait for more feedback and repost then.


================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:210
@@ +209,3 @@
+
+  for (unsigned i = 0, e = ArgLocs.size(); i != e; ++i) {
+    CCValAssign &VA = ArgLocs[i];
----------------
concisified this one as well

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:261-263
@@ +260,5 @@
+  SelectionDAG &DAG = CLI.DAG;
+  SmallVector<ISD::OutputArg, 32> &Outs = CLI.Outs;
+  SmallVector<SDValue, 32> &OutVals = CLI.OutVals;
+  SmallVector<ISD::InputArg, 32> &Ins = CLI.Ins;
+  SDValue Chain = CLI.Chain;
----------------
majnemer wrote:
> Any reason these `SmallVector`s can't be `SmallVectorImpl` ?
doable, but probably better to use auto then...

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:296
@@ +295,3 @@
+
+  for (unsigned i = 0, e = Outs.size(); i != e; ++i) {
+    ISD::ArgFlagsTy Flags = Outs[i].Flags;
----------------
majnemer wrote:
> Would a range-based for loop be more concise?
yep. done

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:345
@@ +344,3 @@
+  // necessary since all emitted instructions must be stuck together.
+  for (unsigned i = 0, e = RegsToPass.size(); i != e; ++i) {
+    Chain = DAG.getCopyToReg(Chain, CLI.DL, RegsToPass[i].first,
----------------
majnemer wrote:
> Would a range-based for loop be more concise?
done

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:362
@@ +361,3 @@
+  SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
+  SmallVector<SDValue, 8> Ops;
+  Ops.push_back(Chain);
----------------
majnemer wrote:
> Consider reserving `RegsToPass.size() + 3` elements to reduce the number of allocations.
that's an overkill. bpf calling convention is set to max of 5 arguments, so 8 here will never trigger a resize.

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:368-370
@@ +367,5 @@
+  // known live into the call.
+  for (unsigned i = 0, e = RegsToPass.size(); i != e; ++i)
+    Ops.push_back(DAG.getRegister(RegsToPass[i].first,
+                                  RegsToPass[i].second.getValueType()));
+
----------------
majnemer wrote:
> Would a range-based for loop be more concise?
done

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:417
@@ +416,3 @@
+  // Copy the result values into the output registers.
+  for (unsigned i = 0; i != RVLocs.size(); ++i) {
+    CCValAssign &VA = RVLocs[i];
----------------
majnemer wrote:
> Would a range-based for loop be more concise?
not here, since OutVals is spoiling clean loop.

================
Comment at: lib/Target/BPF/BPFISelLowering.cpp:458
@@ +457,3 @@
+  // Copy all of the result registers out of their specified physreg.
+  for (unsigned i = 0; i != RVLocs.size(); ++i) {
+    Chain = DAG.getCopyFromReg(Chain, DL, RVLocs[i].getLocReg(),
----------------
majnemer wrote:
> Would a range-based for loop be more concise?
done

================
Comment at: lib/Target/BPF/MCTargetDesc/BPFAsmBackend.cpp:71
@@ +70,3 @@
+  assert(Fixup.getKind() == FK_PCRel_2);
+  *(uint16_t *)&Data[Fixup.getOffset() + 2] = (uint16_t)((Value - 8) / 8);
+}
----------------
majnemer wrote:
> Is this code independent of endianness?
It is in cpu endianness. This 2 bytes is a relative offset in jump instructions. Thanks for checking ;)

================
Comment at: test/CodeGen/BPF/alu8.ll:36-38
@@ +35,5 @@
+; CHECK: xori r2, -1 # encoding: [0xa7,0x02,0x00,0x00,0xff,0xff,0xff,0xff]
+        %1 = xor i8 %b, -1
+        %2 = and i8 %a, %1
+        ret i8 %2
+}
----------------
majnemer wrote:
> Can the leading whitespace of these instructions match the style of the surrounding test case?
good catch! fixed.

http://reviews.llvm.org/D6494

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/