This is to prefer constant return to be setup from imm constant vs.
using a prior reg known to have the same value (by control flow analysis).
While there's nothing wrong with existing approach, it can 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. There's no additional overhead from new codegen anyways.
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]>
(cherry picked from commit e65d38a0dc843d36dd5605c6805645923b44c6a7)
---
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 58d2318f972..42fdc695881 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 c8dad55fd4c..57084ac3fa1 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 c4a06e4f8b8..bbbf7c2e9e6 100644
--- a/gcc/config/bpf/predicates.md
+++ b/gcc/config/bpf/predicates.md
@@ -23,8 +23,7 @@ (define_predicate "reg_or_imm_operand"
(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 00000000000..063a3f60aba
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/const-cost-model-2.c
@@ -0,0 +1,36 @@
+/* Verify 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 00000000000..8cf2fb58492
--- /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.39.5
