Would it be worth putting the logic in an aptly named (private) method?
Something like:
...
private long alignDown(long address, long alignment) {
return address & <whatever>;
}
...
long mapBase = alignDown(address, ps);
…
Cheers,
Mikael
> On May 13, 2019, at 9:14 AM, Andrew Dinn <[email protected]> wrote:
>
> Thank you for looking at the patch.
>
> On 28/04/2019 18:09, Andrew Haley wrote:
>> On 4/25/19 5:34 PM, Andrew Dinn wrote:
>>> long map_base = (address & ~(ps - 1));
>> This looks like a hard way to write
>> long map_base = address & -ps;
>
> My version certainly uses more characters, that is for sure. However, I chose
> (and still prefer) this form for /clarity/. It seems to me to communicate as
> simply as possible what mask is being constructed, granted the starting
> premise that ps is a power of 2 (i.e. has only a single bit set).
>
> It only requires elementary knowledge of binary representations to see:
>
> firstly, that ps-1 clears the original bit and sets all lower bits;
>
> next that ~(ps-1) clears those lower bits and sets all bits from the
> original (inclusive) upwards;
>
> then that the result can be used as a mask to clear those same bottom bits
> from address;
>
> finally that this mask operation is equivalent to rounding down to the
> relevant power of two.
>
> I find your alternative (a tad) less clear because it employs an arithmetic
> operation to achieve the requisite bitwise transformation. That the two
> expressions compute to equivalent results requires some experience and
> understanding of the twos-complement representations of numbers rather than
> basic knowledge of bit fields.
>
> As a gcc hacker 'your mileage may vary' ;-)
>
> Crucially, every compiler we rely on is going to produce the same code in
> both cases.
>
> regards,
>
>
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
