From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZIJlv-0000HX-Re for qemu-devel@nongnu.org; Thu, 23 Jul 2015 12:55:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZIJlu-0007BT-U2 for qemu-devel@nongnu.org; Thu, 23 Jul 2015 12:55:03 -0400 Sender: Paolo Bonzini References: <1437649738-13885-1-git-send-email-peter.maydell@linaro.org> From: Paolo Bonzini Message-ID: <55B11C5C.8050507@redhat.com> Date: Thu, 23 Jul 2015 18:54:52 +0200 MIME-Version: 1.0 In-Reply-To: <1437649738-13885-1-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/5] replace qemu_fls() with pow2ceil()/pow2floor() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: Kevin Wolf , Keith Busch , patches@linaro.org, qemu-block@nongnu.org, "Michael S. Tsirkin" On 23/07/2015 13:08, Peter Maydell wrote: > We have a qemu_fls() function which is just a silly wrapper > around clz32() and which is used in only a handful of places > in the codebase. It turns out that all of those are really > trying to round up or down to a power of 2, which is something > we have utility functions for. This series replaces all > the qemu_fls() calls with pow2ceil() or pow2floor(), and then > removes the now-unused function. The series looks good, but I'd prefer (especially for patch 4 which is in a fast path) if pow2ceil and pow2floor were made inline. BTW, return 1ULL << (64 - nlz); can be implemented as "rotate_right(1ULL, nlz)" (the latter is (1ULL << (64 - nlz)) | (1ULL >> nlz), which only differs in the undefined case nlz == 0). However, this is probably something for the compiler to implement, not really something to do in our sources. Paolo > For the case where you really want to do bit counting rather > than just power-of-2 rounding, you should use the clz/clo > functions directly. > > (I've set myself a little goal of "try to do one cleanup a > day"; that may be a bit ambitious, so we'll see...) > > Peter Maydell (5): > hw/pci: Use pow2ceil() rather than hand-calculation > hw/virtio/virtio-pci: Use pow2ceil() rather than hand-calculation > hw/block/nvme.c: Use pow2ceil() rather than hand-calculation > exec.c: Use pow2floor() rather than hand-calculation > Remove unused qemu_fls function > > exec.c | 4 +--- > hw/block/nvme.c | 2 +- > hw/pci/msix.c | 4 +--- > hw/pci/pci.c | 4 +--- > hw/virtio/virtio-pci.c | 4 +--- > include/qemu-common.h | 1 - > util/cutils.c | 5 ----- > 7 files changed, 5 insertions(+), 19 deletions(-) >