diff mbox series

[RFC,bpf-next,v3,09/12] nfp: bpf: split up the skip flag

Message ID 20181229030923.4804-10-jakub.kicinski@netronome.com
State RFC, archived
Delegated to: BPF Maintainers
Headers show
Series bpf: dead code elimination | expand

Commit Message

Jakub Kicinski Dec. 29, 2018, 3:09 a.m. UTC 
We fail program loading if jump lands on a skipped instruction.
This is for historical reasons, it used to be that we only skipped
instructions optimized out based on prior context, and therefore
the optimization would be buggy if we jumped directly to such
instruction (because the context would be skipped by the jump).

There are cases where instructions can be skipped without any
context, for example there is no point in generating code for:

	 r0 |= 0

We will also soon support dropping dead code, so make the skip
logic differentiate between "optimized with preceding context"
vs other skip types.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/bpf/jit.c  | 36 ++++++++++---------
 drivers/net/ethernet/netronome/nfp/bpf/main.h |  9 +++--
 2 files changed, 26 insertions(+), 19 deletions(-)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 323587dee498..054382b9cbe6 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1266,7 +1266,7 @@  wrp_alu64_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
 	u64 imm = insn->imm; /* sign extend */
 
 	if (skip) {
-		meta->skip = true;
+		meta->flags |= FLAG_INSN_SKIP_NOOP;
 		return 0;
 	}
 
@@ -1296,7 +1296,7 @@  wrp_alu32_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
 	const struct bpf_insn *insn = &meta->insn;
 
 	if (skip) {
-		meta->skip = true;
+		meta->flags |= FLAG_INSN_SKIP_NOOP;
 		return 0;
 	}
 
@@ -3395,7 +3395,7 @@  static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 	int err;
 
 	list_for_each_entry(meta, &nfp_prog->insns, l) {
-		if (meta->skip)
+		if (meta->flags & FLAG_INSN_SKIP_MASK)
 			continue;
 		if (BPF_CLASS(meta->insn.code) != BPF_JMP)
 			continue;
@@ -3439,7 +3439,7 @@  static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
 
 		jmp_dst = meta->jmp_dst;
 
-		if (jmp_dst->skip) {
+		if (jmp_dst->flags & FLAG_INSN_SKIP_PREC_DEPENDENT) {
 			pr_err("Branch landing on removed instruction!!\n");
 			return -ELOOP;
 		}
@@ -3689,7 +3689,7 @@  static int nfp_translate(struct nfp_prog *nfp_prog)
 				return nfp_prog->error;
 		}
 
-		if (meta->skip) {
+		if (meta->flags & FLAG_INSN_SKIP_MASK) {
 			nfp_prog->n_translated++;
 			continue;
 		}
@@ -3737,10 +3737,10 @@  static void nfp_bpf_opt_reg_init(struct nfp_prog *nfp_prog)
 		/* Programs start with R6 = R1 but we ignore the skb pointer */
 		if (insn.code == (BPF_ALU64 | BPF_MOV | BPF_X) &&
 		    insn.src_reg == 1 && insn.dst_reg == 6)
-			meta->skip = true;
+			meta->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
 
 		/* Return as soon as something doesn't match */
-		if (!meta->skip)
+		if (!(meta->flags & FLAG_INSN_SKIP_MASK))
 			return;
 	}
 }
@@ -3755,7 +3755,7 @@  static void nfp_bpf_opt_neg_add_sub(struct nfp_prog *nfp_prog)
 	list_for_each_entry(meta, &nfp_prog->insns, l) {
 		struct bpf_insn insn = meta->insn;
 
-		if (meta->skip)
+		if (meta->flags & FLAG_INSN_SKIP_MASK)
 			continue;
 
 		if (BPF_CLASS(insn.code) != BPF_ALU &&
@@ -3829,7 +3829,7 @@  static void nfp_bpf_opt_ld_mask(struct nfp_prog *nfp_prog)
 		if (meta2->flags & FLAG_INSN_IS_JUMP_DST)
 			continue;
 
-		meta2->skip = true;
+		meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
 	}
 }
 
@@ -3869,8 +3869,8 @@  static void nfp_bpf_opt_ld_shift(struct nfp_prog *nfp_prog)
 		    meta3->flags & FLAG_INSN_IS_JUMP_DST)
 			continue;
 
-		meta2->skip = true;
-		meta3->skip = true;
+		meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
+		meta3->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
 	}
 }
 
@@ -4065,7 +4065,8 @@  static void nfp_bpf_opt_ldst_gather(struct nfp_prog *nfp_prog)
 				}
 
 				head_ld_meta->paired_st = &head_st_meta->insn;
-				head_st_meta->skip = true;
+				head_st_meta->flags |=
+					FLAG_INSN_SKIP_PREC_DEPENDENT;
 			} else {
 				head_ld_meta->ldst_gather_len = 0;
 			}
@@ -4098,8 +4099,8 @@  static void nfp_bpf_opt_ldst_gather(struct nfp_prog *nfp_prog)
 			head_ld_meta = meta1;
 			head_st_meta = meta2;
 		} else {
-			meta1->skip = true;
-			meta2->skip = true;
+			meta1->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
+			meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
 		}
 
 		head_ld_meta->ldst_gather_len += BPF_LDST_BYTES(ld);
@@ -4124,7 +4125,7 @@  static void nfp_bpf_opt_pkt_cache(struct nfp_prog *nfp_prog)
 		if (meta->flags & FLAG_INSN_IS_JUMP_DST)
 			cache_avail = false;
 
-		if (meta->skip)
+		if (meta->flags & FLAG_INSN_SKIP_MASK)
 			continue;
 
 		insn = &meta->insn;
@@ -4210,7 +4211,7 @@  static void nfp_bpf_opt_pkt_cache(struct nfp_prog *nfp_prog)
 	}
 
 	list_for_each_entry(meta, &nfp_prog->insns, l) {
-		if (meta->skip)
+		if (meta->flags & FLAG_INSN_SKIP_MASK)
 			continue;
 
 		if (is_mbpf_load_pkt(meta) && !meta->ldst_gather_len) {
@@ -4246,7 +4247,8 @@  static int nfp_bpf_replace_map_ptrs(struct nfp_prog *nfp_prog)
 	u32 id;
 
 	nfp_for_each_insn_walk2(nfp_prog, meta1, meta2) {
-		if (meta1->skip || meta2->skip)
+		if (meta1->flags & FLAG_INSN_SKIP_MASK ||
+		    meta2->flags & FLAG_INSN_SKIP_MASK)
 			continue;
 
 		if (meta1->insn.code != (BPF_LD | BPF_IMM | BPF_DW) ||
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 941277936475..40291aedd895 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -243,6 +243,13 @@  struct nfp_bpf_reg_state {
 #define FLAG_INSN_IS_JUMP_DST			BIT(0)
 #define FLAG_INSN_IS_SUBPROG_START		BIT(1)
 #define FLAG_INSN_PTR_CALLER_STACK_FRAME	BIT(2)
+/* Instruction is pointless, noop even on its own */
+#define FLAG_INSN_SKIP_NOOP			BIT(3)
+/* Instruction is optimized out based on preceding instructions */
+#define FLAG_INSN_SKIP_PREC_DEPENDENT		BIT(4)
+
+#define FLAG_INSN_SKIP_MASK		(FLAG_INSN_SKIP_NOOP | \
+					 FLAG_INSN_SKIP_PREC_DEPENDENT)
 
 /**
  * struct nfp_insn_meta - BPF instruction wrapper
@@ -271,7 +278,6 @@  struct nfp_bpf_reg_state {
  * @n: eBPF instruction number
  * @flags: eBPF instruction extra optimization flags
  * @subprog_idx: index of subprogram to which the instruction belongs
- * @skip: skip this instruction (optimized out)
  * @double_cb: callback for second part of the instruction
  * @l: link on nfp_prog->insns list
  */
@@ -319,7 +325,6 @@  struct nfp_insn_meta {
 	unsigned short n;
 	unsigned short flags;
 	unsigned short subprog_idx;
-	bool skip;
 	instr_cb_t double_cb;
 
 	struct list_head l;