Skip to content

Commit

Permalink
Merge branch 'dead-code-elimination'
Browse files Browse the repository at this point in the history 
Jakub Kicinski says:

====================
This set adds support for complete removal of dead code.

Patch 3 contains all the code removal logic, patches 2 and 4
additionally optimize branches around and to dead code.

Patches 6 and 7 allow offload JITs to take advantage of the
optimization.  After a few small clean ups (8, 9, 10) nfp
support is added (11, 12).

Removing code directly in the verifier makes it easy to adjust
the relevant metadata (line info, subprogram info).  JITs for
code store constrained architectures would have hard time
performing such adjustments at JIT level.  Removing subprograms
or line info is very hard once BPF core finished the verification.
For user space to perform dead code removal it would have to perform
the execution simulation/analysis similar to what the verifier does.

v3:
 - fix uninitilized var warning in GCC 6 (buildbot).
v4:
 - simplify the linfo-keeping logic (Yonghong).  Instead of
   trying to figure out that we are removing first instruction
   of a subprogram, just always keep last dead line info, if
   first live instruction doesn't have one.
v5:
 - improve comments (Martin Lau).
====================

Signed-off-by: Alexei Starovoitov <[email protected]>
  • Loading branch information
Alexei Starovoitov committed Jan 24, 2019
2 parents bbebce8 + 9a06927 commit 923cefe
Show file tree
Hide file tree
Showing 12 changed files with 1,004 additions and 67 deletions.
42 changes: 22 additions & 20 deletions drivers/net/ethernet/netronome/nfp/bpf/jit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -3182,7 +3182,7 @@ bpf_to_bpf_call(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
wrp_immed_relo(nfp_prog, imm_b(nfp_prog), 0, RELO_IMMED_REL);
} else {
ret_tgt = nfp_prog_current_offset(nfp_prog) + 2;
emit_br(nfp_prog, BR_UNC, meta->n + 1 + meta->insn.imm, 1);
emit_br(nfp_prog, BR_UNC, meta->insn.imm, 1);
offset_br = nfp_prog_current_offset(nfp_prog);
}
wrp_immed_relo(nfp_prog, ret_reg(nfp_prog), ret_tgt, RELO_IMMED_REL);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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 &&
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -4325,7 +4327,7 @@ int nfp_bpf_jit(struct nfp_prog *nfp_prog)
return ret;
}

void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog, unsigned int cnt)
void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog)
{
struct nfp_insn_meta *meta;

Expand Down Expand Up @@ -4353,7 +4355,7 @@ void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog, unsigned int cnt)
else
dst_idx = meta->n + 1 + meta->insn.off;

dst_meta = nfp_bpf_goto_meta(nfp_prog, meta, dst_idx, cnt);
dst_meta = nfp_bpf_goto_meta(nfp_prog, meta, dst_idx);

if (pseudo_call)
dst_meta->flags |= FLAG_INSN_IS_SUBPROG_START;
Expand Down
33 changes: 29 additions & 4 deletions drivers/net/ethernet/netronome/nfp/bpf/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@ 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)
/* Instruction is optimized by the verifier */
#define FLAG_INSN_SKIP_VERIFIER_OPT BIT(5)

#define FLAG_INSN_SKIP_MASK (FLAG_INSN_SKIP_NOOP | \
FLAG_INSN_SKIP_PREC_DEPENDENT | \
FLAG_INSN_SKIP_VERIFIER_OPT)

/**
* struct nfp_insn_meta - BPF instruction wrapper
Expand Down Expand Up @@ -271,7 +281,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
*/
Expand Down Expand Up @@ -319,7 +328,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;
Expand Down Expand Up @@ -407,6 +415,17 @@ static inline bool is_mbpf_div(const struct nfp_insn_meta *meta)
return is_mbpf_alu(meta) && mbpf_op(meta) == BPF_DIV;
}

static inline bool is_mbpf_cond_jump(const struct nfp_insn_meta *meta)
{
u8 op;

if (BPF_CLASS(meta->insn.code) != BPF_JMP)
return false;

op = BPF_OP(meta->insn.code);
return op != BPF_JA && op != BPF_EXIT && op != BPF_CALL;
}

static inline bool is_mbpf_helper_call(const struct nfp_insn_meta *meta)
{
struct bpf_insn insn = meta->insn;
Expand Down Expand Up @@ -457,6 +476,7 @@ struct nfp_bpf_subprog_info {
* @subprog_cnt: number of sub-programs, including main function
* @map_records: the map record pointers from bpf->maps_neutral
* @subprog: pointer to an array of objects holding info about sub-programs
* @n_insns: number of instructions on @insns list
* @insns: list of BPF instruction wrappers (struct nfp_insn_meta)
*/
struct nfp_prog {
Expand Down Expand Up @@ -489,6 +509,7 @@ struct nfp_prog {
struct nfp_bpf_neutral_map **map_records;
struct nfp_bpf_subprog_info *subprog;

unsigned int n_insns;
struct list_head insns;
};

Expand All @@ -505,14 +526,18 @@ struct nfp_bpf_vnic {
};

bool nfp_is_subprog_start(struct nfp_insn_meta *meta);
void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog, unsigned int cnt);
void nfp_bpf_jit_prepare(struct nfp_prog *nfp_prog);
int nfp_bpf_jit(struct nfp_prog *prog);
bool nfp_bpf_supported_opcode(u8 code);

int nfp_verify_insn(struct bpf_verifier_env *env, int insn_idx,
int prev_insn_idx);
int nfp_bpf_finalize(struct bpf_verifier_env *env);

int nfp_bpf_opt_replace_insn(struct bpf_verifier_env *env, u32 off,
struct bpf_insn *insn);
int nfp_bpf_opt_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);

extern const struct bpf_prog_offload_ops nfp_bpf_dev_ops;

struct netdev_bpf;
Expand All @@ -526,7 +551,7 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,

struct nfp_insn_meta *
nfp_bpf_goto_meta(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
unsigned int insn_idx, unsigned int n_insns);
unsigned int insn_idx);

void *nfp_bpf_relo_for_vnic(struct nfp_prog *nfp_prog, struct nfp_bpf_vnic *bv);

Expand Down
9 changes: 8 additions & 1 deletion drivers/net/ethernet/netronome/nfp/bpf/offload.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,9 @@ nfp_prog_prepare(struct nfp_prog *nfp_prog, const struct bpf_insn *prog,

list_add_tail(&meta->l, &nfp_prog->insns);
}
nfp_prog->n_insns = cnt;

nfp_bpf_jit_prepare(nfp_prog, cnt);
nfp_bpf_jit_prepare(nfp_prog);

return 0;
}
Expand Down Expand Up @@ -219,6 +220,10 @@ static int nfp_bpf_translate(struct bpf_prog *prog)
unsigned int max_instr;
int err;

/* We depend on dead code elimination succeeding */
if (prog->aux->offload->opt_failed)
return -EINVAL;

max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
nfp_prog->__prog_alloc_len = max_instr * sizeof(u64);

Expand Down Expand Up @@ -591,6 +596,8 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
const struct bpf_prog_offload_ops nfp_bpf_dev_ops = {
.insn_hook = nfp_verify_insn,
.finalize = nfp_bpf_finalize,
.replace_insn = nfp_bpf_opt_replace_insn,
.remove_insns = nfp_bpf_opt_remove_insns,
.prepare = nfp_bpf_verifier_prep,
.translate = nfp_bpf_translate,
.destroy = nfp_bpf_destroy,
Expand Down
Loading

0 comments on commit 923cefe

Please sign in to comment.