On Sat, Jan 07, 2006 at 12:59:15AM -0800, David S. Miller wrote:
> Why not just make some kind of "negate()" macro that hides away all of
> the typing issues?  Another way to describe the operation is as a
> xor of X with an all-1's bitmask the same size of X.  Maybe that helps
> describe it better?

unsigned short x, y(void);

~x & y();

(unsigned short)~x & y();

(x ^ 0xffff) & y();

are all equivalent, but I wouldn't trust gcc to notice that.  Basically,
the first one is
        r1 = (u32)x;
        r1 = ~r1;
        r2 = y();
        r2 &= 0xffff;
        r3 = r1 & r2;
the second
        r1 = (u32)x;
        r1 = ~r1;
        r1 &= 0xffff;
        r2 = y();
        r2 &= 0xffff;
        r3 = r1 & r2;
and the third - 
        r1 = (u32)x;
        r1 ^= 0xffff;
        r2 = y();
        r2 &= 0xffff;
        r3 = r1 & r2;

I wouldn't particulary hope that gcc gets the second variant optimized as
well as the first one; I _definitely_ would not hope it does the third one
as well as the rest.  So this negate() might be not harmless, especially
since that stuff sits on hot paths.

Basically, I'd rather teach sparse to understand that at least (1) and (2)
are equivalent so it could DTRT itself; that's what the sparse-related chunk
had been about...

BTW, is there any reason why
static inline void ip_eth_mc_map(u32 addr, char *buf)
{
        addr=ntohl(addr);
        buf[0]=0x01;
        buf[1]=0x00;
        buf[2]=0x5e;
        buf[5]=addr&0xFF;
        addr>>=8;
        buf[4]=addr&0xFF;
        addr>>=8;
        buf[3]=addr&0x7F;
}
is not doing just
        unsigned char *p = &addr;
        buf[0]=0x01;
        buf[1]=0x00;
        buf[2]=0x5e;
        buf[3]=p[1]&0x7F;
        buf[4]=p[2];
        buf[5]=p[3];

(and similar for much scarier ip_ib_mc_map() right next to it in
include/net/ip.h)?  I would expect that to give better code, actually...
Mind you, in case of ip_ib_mc_map() I suspect that

static const unsigned char prefix[16] = {
        0,           /* Reserved */
        0xff,         /* Multicast QPN */
        0xff,
        0xff,
        0xff,
        0x12,         /* link local scope */
        0x40,         /* IPv4 signature */
        0x1b
};
        memcpy(buf, prefix, 16);
        addr &= htonl((1<<28)-1);
        memcpy(buf + 16, &addr, 4);

might give even better variant, but that's a separate story...
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to