OK, thanks.
> A common idiom is function returning a constant. If a prio reg is known > to have that same constant value (by control flow analysis), the default > cost model tends to use reg for returning vs. using the constant itself > While there's nothing wrong with this, it sometimes leads to additional > sign-extensions and other corner cases which trip up the bpf kernel verifier > bounds checking for return reg. The verifier improvements are being worked > on but lets adjust the cost model anyways so that constants are favored. > Using a const vs. reg is similar cost. > > For accompanying test const-cost-model.c, at -O2 > > Before | After > -------------------------------+------------------------------ > r1 = 4 | r1 = 4 > call bpf_copy_from_user_str | call bpf_copy_from_user_str > r1 = (s32) r0 | ... > ... | r1 = 4 > call bpf_copy_from_user_str | call bpf_copy_from_user_str > > This causes 4 additioanl bpf kernel selftests to pass (as of v7.1) > > | -Summary: 596/5387 PASSED, 123 SKIPPED, 114 FAILED > | +Summary: 600/5422 PASSED, 123 SKIPPED, 110 FAILED > | #77 cgroup_xattr:OK > | #112 exe_ctx:OK > | #220 mem_rdonly_untrusted:OK > | #574 verifier_global_ptr_args:OK > > gcc/ChangeLog: > > * config/bpf/bpf.cc (bpf_rtx_costs): set CONST_INT 0 insns. > * config/bpf/bpf.h (BPF_IMM32_P): New helper macro. > * config/bpf/predicates.md (imm32_operand): Use new macro. > > gcc/testsuite/ChangeLog: > > * gcc.target/bpf/const-cost-model.c: New test prefers const. > * gcc.target/bpf/const-cost-model-2.c: New test prefers const. > > Signed-off-by: Vineet Gupta <[email protected]> > --- > gcc/config/bpf/bpf.cc | 5 +++ > gcc/config/bpf/bpf.h | 5 +++ > gcc/config/bpf/predicates.md | 3 +- > .../gcc.target/bpf/const-cost-model-2.c | 36 ++++++++++++++++ > .../gcc.target/bpf/const-cost-model.c | 42 +++++++++++++++++++ > 5 files changed, 89 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/bpf/const-cost-model-2.c > create mode 100644 gcc/testsuite/gcc.target/bpf/const-cost-model.c > > diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc > index 58d2318f972b..42fdc695881a 100644 > --- a/gcc/config/bpf/bpf.cc > +++ b/gcc/config/bpf/bpf.cc > @@ -625,6 +625,11 @@ bpf_rtx_costs (rtx x, > *total = COSTS_N_INSNS (1); > return false; > > + case CONST_INT: > + /* Signed 32-bit imm is free, wider needs an extra LD_IMM64. */ > + *total = BPF_IMM32_P (x) ? 0 : COSTS_N_INSNS (1); > + return true; > + > default: > return false; > } > diff --git a/gcc/config/bpf/bpf.h b/gcc/config/bpf/bpf.h > index c8dad55fd4c4..57084ac3fa15 100644 > --- a/gcc/config/bpf/bpf.h > +++ b/gcc/config/bpf/bpf.h > @@ -526,4 +526,9 @@ enum reg_class > > #define ASSEMBLER_DIALECT ((int) asm_dialect) > > +/*** Utilitiy/helpers. */ > + > +#define BPF_IMM32_P(X) \ > + (CONST_INT_P (X) && (INTVAL (X) == (HOST_WIDE_INT) (int32_t) INTVAL (X))) > + > #endif /* ! GCC_BPF_H */ > diff --git a/gcc/config/bpf/predicates.md b/gcc/config/bpf/predicates.md > index c4a06e4f8b8c..bbbf7c2e9e61 100644 > --- a/gcc/config/bpf/predicates.md > +++ b/gcc/config/bpf/predicates.md > @@ -23,8 +23,7 @@ > (match_operand 0 "register_operand"))) > > (define_predicate "imm32_operand" > - (ior (and (match_code "const_int") > - (match_test "IN_RANGE (INTVAL (op), 0, 0xffffffff)")) > + (ior (match_test "BPF_IMM32_P (op)") > (match_code "symbol_ref,label_ref,const"))) > > (define_predicate "core_imm_operand" > diff --git a/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c > b/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c > new file mode 100644 > index 000000000000..ab37de27b566 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c > @@ -0,0 +1,36 @@ > +/* Verift that function return values are const 0 and 1 and not reg. > + Extracted from BPF selftest sockopt_multi.c */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcpu=v4" } */ > + > +struct bpf_sockopt { > + int level, optname, optlen, retval; > + void *optval_end, *optval; > +}; > + > +typedef unsigned char u8; > + > +int getsockopt_child(struct bpf_sockopt *ctx) > +{ > + u8 *optval_end = ctx->optval_end; > + u8 *optval = ctx->optval; > + > +// if (ctx->optname != 1) > +// goto out; > + > + if (ctx->level != 0 || ctx->optname != 1) > + goto out; > + > + if (optval[0] != 0x80) > + return 0; > + > + ctx->retval = 0; > + return 1; > + > +out: > + return 1; > +} > + > +/* { dg-final { scan-assembler-not {\*\(u32 \*\) \(r.\+12\) = r.} } } */ > +/* { dg-final { scan-assembler-times {\*\(u32 \*\) \(r.\+12\) = 0} 1 } } */ > diff --git a/gcc/testsuite/gcc.target/bpf/const-cost-model.c > b/gcc/testsuite/gcc.target/bpf/const-cost-model.c > new file mode 100644 > index 000000000000..8cf2fb584922 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/bpf/const-cost-model.c > @@ -0,0 +1,42 @@ > +/* Verify that arg const 4 to 2nd call is const and not reg r0 which is > + ret value of first call. > + Extracted from BPF selftest attach_probe.c */ > + > +/* { dg-do compile } */ > +/* { dg-options "-O2 -mcpu=v4" } */ > + > +extern int bpf_copy_from_user_str(int dst__sz); > + > +_Bool verify_sleepable_user_copy_str(void) > +{ > + int ret; > + char data_short_pad[4]; > + > + ret = bpf_copy_from_user_str(sizeof(data_short_pad)); > + > + if (ret != 4) > + return false; > + > + ret = bpf_copy_from_user_str(sizeof(data_short_pad)); > + > + if (ret != 4) > + return false; > + > + return true; > +} > + > +/* Original generated code > + r1 = 4 > + call bpf_copy_from_user_str > + r1 = (s32) r0 > + ... > + call bpf_copy_from_user_str > + > + Now generated > + r1 = 4 > + call bpf_copy_from_user_str > + ... > + r1 = 4 > + call bpf_copy_from_user_str */ > + > +/* { dg-final { scan-assembler-times {r1 = 4} 2 } } */
