bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] selftests/bpf: adjust strobemeta loop to satisfy latest clang
@ 2019-09-25 18:52 Andrii Nakryiko
  2019-09-25 20:33 ` Daniel Borkmann
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2019-09-25 18:52 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Some recent changes in latest Clang started causing the following
warning when unrolling strobemeta test case main loop:

  progs/strobemeta.h:416:2: warning: loop not unrolled: the optimizer was
  unable to perform the requested transformation; the transformation might
  be disabled or specified as part of an unsupported transformation
  ordering [-Wpass-failed=transform-warning]

This patch simplifies loop's exit condition to depend only on constant
max iteration number (STROBE_MAX_MAP_ENTRIES), while moving early
termination logic inside the loop body. The changes are equivalent from
program logic standpoint, but fixes the warning. It also appears to
improve generated BPF code, as it fixes previously failing non-unrolled
strobemeta test cases.

Cc: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/progs/strobemeta.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/strobemeta.h b/tools/testing/selftests/bpf/progs/strobemeta.h
index 8a399bdfd920..067eb625d01c 100644
--- a/tools/testing/selftests/bpf/progs/strobemeta.h
+++ b/tools/testing/selftests/bpf/progs/strobemeta.h
@@ -413,7 +413,10 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,
 #else
 #pragma unroll
 #endif
-	for (int i = 0; i < STROBE_MAX_MAP_ENTRIES && i < map.cnt; ++i) {
+	for (int i = 0; i < STROBE_MAX_MAP_ENTRIES; ++i) {
+		if (i >= map.cnt)
+			break;
+
 		descr->key_lens[i] = 0;
 		len = bpf_probe_read_str(payload, STROBE_MAX_STR_LEN,
 					 map.entries[i].key);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] selftests/bpf: adjust strobemeta loop to satisfy latest clang
  2019-09-25 18:52 [PATCH bpf] selftests/bpf: adjust strobemeta loop to satisfy latest clang Andrii Nakryiko
@ 2019-09-25 20:33 ` Daniel Borkmann
  2019-09-25 20:35   ` Andrii Nakryiko
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2019-09-25 20:33 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, andrii.nakryiko, kernel-team

On Wed, Sep 25, 2019 at 11:52:05AM -0700, Andrii Nakryiko wrote:
> Some recent changes in latest Clang started causing the following
> warning when unrolling strobemeta test case main loop:
> 
>   progs/strobemeta.h:416:2: warning: loop not unrolled: the optimizer was
>   unable to perform the requested transformation; the transformation might
>   be disabled or specified as part of an unsupported transformation
>   ordering [-Wpass-failed=transform-warning]
> 
> This patch simplifies loop's exit condition to depend only on constant
> max iteration number (STROBE_MAX_MAP_ENTRIES), while moving early
> termination logic inside the loop body. The changes are equivalent from
> program logic standpoint, but fixes the warning. It also appears to
> improve generated BPF code, as it fixes previously failing non-unrolled
> strobemeta test cases.
> 
> Cc: Alexei Starovoitov <ast@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Sounds like a clang regression? Was that from an official release?

Applied.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf] selftests/bpf: adjust strobemeta loop to satisfy latest clang
  2019-09-25 20:33 ` Daniel Borkmann
@ 2019-09-25 20:35   ` Andrii Nakryiko
  0 siblings, 0 replies; 3+ messages in thread
From: Andrii Nakryiko @ 2019-09-25 20:35 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov, Kernel Team

On Wed, Sep 25, 2019 at 1:33 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On Wed, Sep 25, 2019 at 11:52:05AM -0700, Andrii Nakryiko wrote:
> > Some recent changes in latest Clang started causing the following
> > warning when unrolling strobemeta test case main loop:
> >
> >   progs/strobemeta.h:416:2: warning: loop not unrolled: the optimizer was
> >   unable to perform the requested transformation; the transformation might
> >   be disabled or specified as part of an unsupported transformation
> >   ordering [-Wpass-failed=transform-warning]
> >
> > This patch simplifies loop's exit condition to depend only on constant
> > max iteration number (STROBE_MAX_MAP_ENTRIES), while moving early
> > termination logic inside the loop body. The changes are equivalent from
> > program logic standpoint, but fixes the warning. It also appears to
> > improve generated BPF code, as it fixes previously failing non-unrolled
> > strobemeta test cases.
> >
> > Cc: Alexei Starovoitov <ast@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Sounds like a clang regression? Was that from an official release?

It does, but I didn't dig deep enough to figure out what exactly
caused this. The version I used was latest Clang 10 built from
sources. Might be worth-while to investigate this further to prevent
some other unexpected breakages for user programs.

>
> Applied.

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-09-25 20:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 18:52 [PATCH bpf] selftests/bpf: adjust strobemeta loop to satisfy latest clang Andrii Nakryiko
2019-09-25 20:33 ` Daniel Borkmann
2019-09-25 20:35   ` Andrii Nakryiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).