Messages in this thread | | | From | Rasmus Villemoes <> | Subject | Re: [PATCH v2 1/3] lib: find_*_bit reimplementation | Date | Thu, 05 Feb 2015 15:51:32 +0100 |
| |
On Thu, Feb 05 2015, Yury <yury.norov@gmail.com> wrote:
> On 02.02.2015 15:56, Rasmus Villemoes wrote: >> On Mon, Feb 02 2015, "George Spelvin" <linux@horizon.com> wrote: >> >>> Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: >>>> ... and this be part of _find_next_bit? Can find_next_bit not be simply >>>> 'return _find_next_bit(addr, size, offset, 1);', and similarly for >>>> find_next_zero_bit? Btw., passing true and false for the boolean >>>> parameter may be a little clearer. >>> Looking at the generated code, it would be better to replace the boolean >>> parameter with 0ul or ~0ul and XOR with it. The same number of registers, >>> and saves a conditional branch. >> Nice trick. When I compiled it, gcc inlined _find_next_bit into both its >> callers, making the conditional go away completely. That was with gcc >> 4.7. When I try with 5.0, I do see _find_next_bit being compiled >> separately. >> >> With the proposed change, 4.7 also makes find_next{,_zero}_bit wrappers >> for _find_next_bit, further reducing the total size, which is a good >> thing. And, if some other version decides to still inline it, it >> should then know how to optimize the xor with 0ul or ~0ul just as well >> as when the conditional was folded away. >> >> Yury, please also incorporate this in the next round. >> >> Rasmus >> > Ok.
Good.
> What are you thinking about joining _find_next_bit and > _find_next_bit_le?
I don't think that should be done right now, if at all. The series is pretty close to getting my Reviewed-by; I'd prefer not to start over.
Rasmus
| |