From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, David Faust <david.faust@oracle.com>,
Cupertino Miranda <cupertino.miranda@oracle.com>
Subject: Re: [PATCH bpf-next] bpf: make list_for_each_entry portable
Date: Fri, 10 May 2024 10:26:58 +0200 [thread overview]
Message-ID: <874jb62ht9.fsf@oracle.com> (raw)
In-Reply-To: <CAADnVQJRpCX+vmwCu3xYz+V4Bq1gn3vnCAZk3CAJcB3KUq_-Cg@mail.gmail.com> (Alexei Starovoitov's message of "Thu, 9 May 2024 14:48:58 -0700")
> On Thu, May 9, 2024 at 1:47 AM Jose E. Marchesi
> <jose.marchesi@oracle.com> wrote:
>> +/* A `break' executed in the head of a `for' loop statement is bound
>> + to the current loop in clang, but it is bound to the enclosing loop
>> + in GCC. Note both compilers optimize the outer loop out with -O1
>> + and higher. This macro shall be used to annotate any loop that
>> + uses cond_break within its header. */
>> +#ifdef __clang__
>> +#define __compat_break
>> +#else
>> +#define __compat_break for (int __control = 1; __control; --__control)
>> +#endif
> ..
>> + __compat_break
>> for (i = zero; i < cnt; cond_break, i++) {
>> struct elem __arena *n = bpf_alloc(sizeof(*n));
>
> This is too ugly. It ruins the readability of the code.
> Let's introduce can_loop macro similar to cond_break
> that returns 0 or 1 instead of break/continue and use it as:
>
> for (i = zero; i < cnt && can_loop; i++) {
>
> pw-bot: cr
I went with the ugliness because I was trying to avoid rewriting the
loops in the tests, assuming the tests were actually testing using
cond_break in these particular locations would result in a particular
number of iterations.
The loops
for (i = zero; i < cnt; cond_break, i++) BODY
and
for (i = zero; i < cnt && can_loop; i++) BODY
are not equivalent if can_loop implements the same logic than
cond_break.
The may_goto instructions are somehow patched at run-time, and in a
predictable way since the tests are checking for explicit iteration
counts, right?
next prev parent reply other threads:[~2024-05-10 8:27 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 8:46 [PATCH bpf-next] bpf: make list_for_each_entry portable Jose E. Marchesi
2024-05-09 21:48 ` Alexei Starovoitov
2024-05-10 8:26 ` Jose E. Marchesi [this message]
2024-05-10 17:03 ` Alexei Starovoitov
2024-05-10 17:16 ` Jose E. Marchesi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874jb62ht9.fsf@oracle.com \
--to=jose.marchesi@oracle.com \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=cupertino.miranda@oracle.com \
--cc=david.faust@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.