From owner-freebsd-current Mon Nov 15 20:18:30 1999 Delivered-To: freebsd-current@freebsd.org Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by hub.freebsd.org (Postfix) with ESMTP id 174FC14EC0 for ; Mon, 15 Nov 1999 20:18:23 -0800 (PST) (envelope-from bde@zeta.org.au) Received: from p123-ts5.syd2.zeta.org.au (beefcake.zeta.org.au [203.26.10.12]) by mailman.zeta.org.au (8.8.7/8.8.7) with ESMTP id PAA02289; Tue, 16 Nov 1999 15:24:29 +1100 Date: Tue, 16 Nov 1999 15:17:43 +1100 (EST) From: Bruce Evans X-Sender: bde@alphplex.bde.org To: Pierre Beyssac Cc: Garrett Wollman , Sheldon Hearn , freebsd-current@FreeBSD.ORG Subject: Re: egcs -O breaks ping.c:in_cksum() In-Reply-To: <19991115194357.T28348@enst.fr> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG On Mon, 15 Nov 1999, Pierre Beyssac wrote: > On Mon, Nov 15, 1999 at 01:35:15PM -0500, Garrett Wollman wrote: > > If, rather than casting pointers, the code used a union (containing > > one u_int16_t and one array[2] of u_int8_t), the compiler would have > > enough information to know about the aliases. > > You're right, this seems to work even with optimization turned on. > If nobody objects, I'll commit it. > > --- ck.c.old Mon Nov 15 19:41:35 1999 > +++ ck.c Mon Nov 15 19:39:43 1999 > @@ -13,7 +13,10 @@ > register int nleft = len; > register u_short *w = addr; > int sum = 0; > - volatile u_short answer = 0; > + union { > + u_int16_t us; > + u_int8_t uc[2]; > + } answer; This has indentation bugs. ping.c still assumes that u_short is u_int16_t everywhere else. > > /* > * Our algorithm is simple, using a 32 bit accumulator (sum), we add > @@ -27,15 +30,16 @@ > > /* mop up an odd byte, if necessary */ > if (nleft == 1) { > - *(u_char *)(&answer) = *(u_char *)w ; > - sum += answer; > + answer.uc[0] = *(u_char *)w ; > + answer.uc[1] = 0; > + sum += answer.us; This `answer' variable has nothing to do with the final `answer' variable. The latter should not be a union. The original code apparently reuses `answer' to do manual register allocation for ancient compilers. Perhaps the above should be written as: sum += ntohs(*(u_char *)w << 8); to avoid the undefined union access (answer.us). I think this works on all systems, but it is a pessimisation on some little-endian systems including i386's (on i386's, ntohs() is inline, but it is inline asm so the compiler can't see that it just reverses the shift). Bruce To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message