core: thread_flags: optimize thread_flags_wait_one, fix doxygen#5200
core: thread_flags: optimize thread_flags_wait_one, fix doxygen#5200miri64 merged 2 commits intoRIOT-OS:masterfrom
Conversation
kaspar030
added
Type: enhancement
|
ACK |
kaspar030
added
the
CI: ready for build
core/thread_flags.c
Outdated
| thread_t *me = (thread_t*) sched_active_thread; | ||
| unsigned tmp = me->flags & mask; | ||
| return _thread_flags_clear_atomic(me, thread_flags_clear(1 << bitarithm_lsb(tmp))); | ||
| tmp &= -tmp; |
There was a problem hiding this comment.
Better ~tmp + 1 so it does not break for architectures that don't use two's complement. At least in theory there could be such systems. Even with -O0 gcc understands and optimizes the code, so there is no drawback.
There was a problem hiding this comment.
hmm sounds sensible. If someone wants to compile for a PDP, then it might choose one's complement instead of 2's
kaspar030
force-pushed
the
optimize_thread_flags
branch
from
7384f52 to
46e49fa
Compare
@cgundogan ACK holds? |
|
Just to make those PDP users happy. ;) |
|
yes ACK upholds |
|
do we need a second one or is the change minor enough? |
core/thread_flags.c
Outdated
| return _thread_flags_clear_atomic(me, thread_flags_clear(1 << bitarithm_lsb(tmp))); | ||
| thread_flags_t tmp = me->flags & mask; | ||
| /* clear all but least significant bit */ | ||
| tmp &= ~tmp + 1; |
There was a problem hiding this comment.
how would you like them?
There was a problem hiding this comment.
Yeah, how is correctly? "not" has precedence over "plus", so do I parenthise (~tmp) +1?
There was a problem hiding this comment.
Well, that was my reason to ask for parentheses. If I don't how this evaluated, I cannot judge if it is correct. ;)
There was a problem hiding this comment.
There was a problem hiding this comment.
Why not creating an additional Github account and acknowledge your own PRs?
There was a problem hiding this comment.
Could someone else please step in and either ACK this or explain which parens help understanding the expression?
There was a problem hiding this comment.
I guess tmp &= (~tmp + 1); would be okay.
|
I could give a second ACK if my comment is addressed. |
kaspar030
force-pushed
the
optimize_thread_flags
branch
from
92fb456 to
412b363
Compare
|
|
ACK |
|
Three ACKs (by two people) and a happy Murdock. Let's go. |
5 participants
No need to call expensive bitarithm_lsb() for just keeping the most significant bit in thread_flags_wait_one(). Also fix doxygen explaining wrong order.