lkml.org 
[lkml]   [2017]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] intel: Use upper_32_bits and lower_32_bits
From
Date
On Mon, 2017-01-09 at 12:55 +0000, David Laight wrote:
> From: Joe Perches
> > Sent: 07 January 2017 18:33
> > Shifting and masking various types can be made a bit
> > simpler to read by using the available kernel macros.
>
> ...
> > - ew32(TDBAH, (tdba >> 32));
> > - ew32(TDBAL, (tdba & 0x00000000ffffffffULL));
> > + ew32(TDBAH, upper_32_bits(tdba));
> > + ew32(TDBAL, lower_32_bits(tdba));
>
> Personally I find the original code easier to understand
> since I don't have to look up another silly macro.

It's already a pretty common usage and I believe
the naming is fairly obvious. Also you don't have
to count the "f" characters to see how many bits
are being used. After about 6 consecutive chars,
it can be error prone.

The leading zeros? ugh. The ULL too.

$ git grep -w -E "upper_32_bits|lower_32_bits" | wc -l
1569

> I'd normally not even explicitly mask the low bits
> relying on the implicit truncation of the assignment.

Relying on implicit behaviors can be noisy when
compilers complain about implicit conversions and
truncations.

> At least modern compilers aren't stupid enough to add two
> 'mask with 0xff' instructions for:
> *uchar_ptr = (unsigned char)(foo & 0xff);

I agree it's visual noise.

\
 
 \ /
  Last update: 2017-01-09 14:39    [W:0.334 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site