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 } } */
--
2.55.0
