On Mon, Dec 12, 2011 at 6:06 AM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: > On 11/12/11 20:07, Christoffer Dall wrote: >> On Dec 11, 2011, at 2:48 PM, Peter Maydell <peter.maydell@xxxxxxxxxx> wrote: >> >>> On 11 December 2011 19:30, Christoffer Dall >>> <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote: >>>> On Sun, Dec 11, 2011 at 11:03 AM, Peter Maydell >>>> <peter.maydell@xxxxxxxxxx> wrote: >>>>> Removing the mask would be wrong since the irq field here >>>>> is encoding both cpu number and irq-vs-fiq. The default is >>>>> just an unreachable condition. (Why are we using % here >>>>> rather than the obvious bit operation, incidentally?) >>>>> >>>> right, I will remove the default case. >>>> >>>> I highly doubt that the difference in using a bitop will be measurably >>>> more efficient, but if you feel strongly about it, I can change it to >>>> a shift and bitwise and, which I assume is what you mean by the >>>> obvious bit operation? I think my CS background speaks for using %, >>>> but whatever. >>> >>> Certainly the compiler ought to be able to figure out the >>> two are the same thing; I just think "irq & 1" is more readable >>> than "irq % 2" (because it's being clear that it's treating the >>> variable as a pile of bits rather than an integer). This is >>> bikeshedding rather, though, and style issues in kernel code >>> are a matter for the kernel folk. So you can ignore me :-) >>> >> Well, if it was just "irq & 1", then I hear you, but it would be "(irq >>>> cpu_idx) & 1" which I don't think is more clear. >> >> But yes let's see what the kernel folks say. > > The general consensus is to use bit operations rather than arithmetic. > The compiler will usually convert the "% 2" pattern into a shift, but I > tend to agree with Peter on the readability of the thing. When encoding > multiple information in a word, bit operations should be used, as they > make it obvious which part of the word contains the bit you're > interested in. > > But I've probably been corrupted by working with HW guys for a bit too > long... ;-) > > ok, ok, I'll change it to a bit op. Can't wait for the dazzling performance improvement ;) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
