On Thu, 2008-04-10 at 22:43 +0100, David Howells wrote:
> Harvey Harrison <harvey.harrison@xxxxxxxxx> wrote:
>
> > +static inline u16 __get_unaligned_le16(const u8 *p)
> > +{
> > + return (u16)(p[0] | p[1] << 8);
> > +}
>
> You shouldn't need these casts. return is going to cast it anyway.
>
> Actually, you probably _ought_ to have casts, but it should look like this:
>
> return (u16)p[0] | (u16)p[1] << 8;
I've been looking at that thinking I needed something different, I
believe it is ok as u8 will expand to int when shifted... correct? Or
do I actually need the cast on each p[] term...anyone?
>
> You are shifting an 8-bit value left by 8 bits, so the compiler may be at
> liberty to instruct the RHS to end up zero.
>
> I presume the compiler is guaranteed not to merge the two memory accesses? It
> can't seem to make it do so, though I seem to remember there were cases where
> it did, though I can't reproduce them. I assume that's why you're passing in
> a u8 pointer and not a u16/u32/u64 pointer.
Yes, that is the reason. The implementation is nearly identical to the
existing arm version in-tree (minus the register keywords of course).
Harvey
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
