as in bug report

>Submitter-Id:  net
>Originator:    Marco Munari	http://mm.homeunix.org:8380/marcomunari/
>Organization:	allerta.it	http://www.allerta.it/
>Synopsis:
	uninitialized int assigned as short possibly inhibit ARP to work properly
	(is a gcc bug, but kernel code (for all architectures) facilitate it)
	isolated a compilation bug in i386 architectures when compiling
        with options  -Os -march=i686
>Severity:
	serious
>Priority:
	medium
>Category:
	i386
>Class:
	sw-bug
>Release:
	ALL (cvs current, since 18-Oct-95)
>Environment:
System      : OpenBSD 3.8
Architecture: OpenBSD.i386
Machine     : i386

>Description:

[before gnats didn't assign a PR number, because ``Category'' was double]
for clarification to those new to the bug, the original mail is also here:
http://marc.theaimsgroup.com/?l=openbsd-bugs&m=114083145723359&w=2
and the previous thread connected to this PR was mirrored here:
http://marc.theaimsgroup.com/?t=114083155500001&r=1&w=2

PS: now i can say that the main bug is in gcc-3.3.5 source code
    (config/i386/i386.{md,c}), anyway you can consider to fix also an
    unnatural code in the OpenBSD kernel if_ether.c.

On Sun, 26 Feb 2006 21:26:53 +0100 (CET), Otto Moerbeek  includes:
}On Sat, 25 Feb 2006, Marco Munari wrote:
...
}>>Category:
}>kernel, system
...
}>
}>In one of my OpenBSD systems ARP didn't work, the reason -common to OpenBSD-
}>was the following:
}>
}>I noticed that op in if_ether.c is more natural of type u_int16_t,
}>insted of int
}>
}>file,version:line:
}>sys/netinet/if_ether.c,1.1:531: 	int op;
}>                                      ===
}>sys/netinet/if_ether.c,1.1:534: 	op = ntohs(ea->arp_op);
}>
}>it heppen that op was -741081087 (= D3D40001) instead of 1
}>(in some 16-bit-also architectures such as AMD64, the highter
}>half of 32 bit is never initialized, but used in comparisons)
}>
}>
}>code generated by gcc:
}>
}>d025e16c:       0f b7 41 06             movzwl 0x6(%ecx),%eax
}>
}>d025e170:       86 c4                   xchg   %al,%ah
}>                                       ;rorw $8,%ax in ~i486+ w pipelines
}>                                       ;(xchg insted of rotate by 8 bits
}>                                       ; is efficient only in old CPU
}>                                       ; w/o pipelines)
}>
}>d025e172:       66 89 45 b0             mov    %ax,0xffffffb0(%ebp)
}>
}>last mov instruction is inapropriate for signed int,
}>this seems a compiler(gcc-3.3.5) bug, above instruction should be:
}>movzwl %ax,0xffffffb0(%ebp)
}>

On Sun, 26 Feb 2006 21:26:53 +0100 (CET), Otto Moerbeek  wrote:

}This code does not correspond to the object code I find in my kernel.

Yes, I changed ntohs() function, the standard is  rol $0x8,%ax  instead of xchg,
can you include the uncorrespondent piece of code generated by your
kernel/compiler (just to avoid at last doubts about my compiled gcc...)


}Two possible explanations:

}- You are running into a bug that occurs on amd64 in i386 mode,

this was my first doubt, but now it's evident to me that there is
a case for gcc-3.3.5 in OpenBSD that generate inappropriate code
in conversion from u_int16_t to int for 686 architectures.

This inappropriate generated code maybe reveal it's error in AMD64 in i386 mode
because of possible stack run-time change (any interrupt push
registers to stack), or in this architecture/mode the kernel stack
results not initialized to zero,
but the opcode generated is wrong with some compilation flags
because the assembly code do not assign zero to the hi half of the dword,
it generate just the wrong instruction (``mov'' instead of ``movzwl'')

}- You have tweaked compilations options.
}I think the latter is more likely. We warn agains using non-standard
}compilation options for a reason.

yes, I have tweaked the compilation options.
so I've determined that the following gcc 3.3.5 flags
 -Os -march=i686
produces the wrong code.
I know the suggestion to don't change, don't tweak etc. but in my
philosophy progress is due to perfect exception, not to conservation.
(obviously having control of what we do)

the following three lines

sys/netinet/if_ether.c,1.1:531: 	int op;
sys/netinet/if_ether.c,1.1:534: 	op = ntohs(ea->arp_op);
sys/netinet/if_ether.c:534:	if ((op != ARPOP_REQUEST) && (op != ARPOP_REPLY))

compiled with
cc -fno-builtin-printf -fno-builtin-log -O2 \
 -pipe -Os -momit-leaf-frame-pointer -mno-stack-arg-probe \
 -nostdinc -I. -I../../../../arch -I../../../.. \
 -DDIAGNOSTIC -DKTRACE -DCRYPTO -DSYSVMSG -DSYSVSEM -DSYSVSHM \
 -DCOMPAT_35 -DCOMPAT_43 -DLKM -DFIFO -DINET -DI686_CPU -DDEBUG \
 -D_KERNEL -Di386 -c ../../../../netinet/if_ether.c

generates the following proper code

     750:   66 8b 41 06     mov    0x6(%ecx),%ax	; AX = ea->arp_op
     754:   66 c1 c0 08     rol    $0x8,%ax
     758:   0f b7 c0        movzwl %ax,%eax
     75b:   89 45 b0        mov    %eax,0xffffffb0(%ebp)
     75e:   48              dec    %eax
     75f:   31 ff           xor    %edi,%edi
     761:   31 f6           xor    %esi,%esi
     763:   83 f8 01        cmp    $0x1,%eax
     766:   76 14           jbe    77c <in_arpinput+0x56>
     ...

but adding -march=i686 option (keeping -Os and the other options),
 the code generated results (wrong code):

     758:   0f b7 41 06     movzwl 0x6(%ecx),%eax	; EAX = ea->arp_op
     75c:   66 c1 c0 08     rol    $0x8,%ax
     760:   66 89 45 b0     mov    %ax,0xffffffb0(%ebp)	; first error
                            ==========
     764:   8b 45 b0        mov    0xffffffb0(%ebp),%eax	; double error
     767:   48              dec    %eax		; eax[31..16] is undefined here
     768:   83 f8 01        cmp    $0x1,%eax
     76b:   76 14           jbe    781 <in_arpinput+0x57>
     ...

 the signed comparison jump (jbe) is right or wrong depending on
 the ``undefined`` eax[31] and eax[30..16] part of register EAX.

the appropriate instruction here above at address 0x760
should be   movzwl %ax,0xffffffb0(%ebp)
else the comparison is with an uninitialized half int in the stack area.

The code is unequivocally wrong when -march=i686 -Os is used.


with -march=i386, i486 or i586 insted of i686 the code is OK.
the code generated is good also without -Os and with -march=i686

     931:       0f b7 41 06             movzwl 0x6(%ecx),%eax
     935:       66 c1 c0 08             rol    $0x8,%ax
     939:       8d 58 ff                lea    0xffffffff(%eax),%ebx
     93c:       83 fb 01                cmp    $0x1,%ebx


the error is in the optimization code of gcc 3.3.5 when
 TARGET_PENTIUMPRO is true and optimize_size is true
in i386.c  (/usr/src/gnu/usr.bin/gcc/gcc/config/i386/i386.c).

I never changed gcc optimization source code till now,
if there is no one smart with gcc in OpenBSD development team
inspired by the desire to fix it, I can have a look to this
intermediate gcc version (3.3.5). (last gcc version is 4.1.0).


I noticed with the search in
 http://gcc.gnu.org/cgi-bin/search.cgi?q=movzwl+mov&cmd=Search%21
a similar bug report related ~about~ to the same period of gcc (3.3.x),
introduced by http://gcc.gnu.org/ml/gcc-bugs/2004-08/msg03118.html
here:  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17213




}Now I agree that from a stylistic point of view, you could prefer
}using a unsigned short here, but as far as i can tell, the code is not
}wrong.

Yes, the kernel C code is not wrong, but as i said it facilitate
a bug of gcc to emerge.

}The variable op will be promoted to int anyway when compared to
}the constants,

Yes, in theory should be.

}	-Otto



}On Sat, 25 Feb 2006, Marco Munari wrote (continuation):
}>
}>last mov instruction is inapropriate for signed int,
}>this seems a compiler(gcc-3.3.5) bug, above instruction should be:
}>movzwl %ax,0xffffffb0(%ebp)
}>
}>I'll have the opportunity to verify again more carefully the gcc generated
}>code (currently I'm physically 600km far from the fixed machine that
}>didn't work properly),  the ``optimization'' flag -O2 produced
}>very different code each minimal changment in the source, I have some
}>dubts for now, except that
}>(int)op was -741081087  and  ARP didn't work for the wrong op
}>assignment in that machine.
}>
}>PS: an explicit assigment op = (int)ntohs(ea->arp_op); seem to produce
}>also the wrong code (but I didn't run the code in this second case).
}>op = 0x0000ffff & ntohs(ea->arp_op);  // solves (but is undesirable)

I re-verified watching more carefully the code and I confirm, it's a
gcc bug for i386, for easier reading and to continue the proper bug report
i remove the "}>" in the following lines i wrote:



to be precise my sys/arch/i386/include/endian.h contains:
#define __swap16md(x) ({                                                \
u_int16_t __swap16md_x = (x);                                   \
__asm ("xchgb %b1, %h1" : "+Q" (__swap16md_x)); /* changed */   \
__swap16md_x;                                                   \
})
but the result is the same with the original line
__asm ("rolw $8, %w1" : "+r" (__swap16md_x));			\



op nature is u_int16_t as the field ar_op alias arp_op in the structure.

if_ether.h:
/*
* Ethernet Address Resolution Protocol.
*
* See RFC 826 for protocol description.  Structure below is adapted
* to resolving internet addresses.  Field names used correspond to
* RFC 826.
*/
struct	ether_arp {
struct	 arphdr ea_hdr;			/* fixed-size header */
...
};
...
#define	arp_op	ea_hdr.ar_op
...

if_arp.h:
struct	arphdr {
...
u_int16_t ar_op;	/* one of: */
=========
#define	ARPOP_REQUEST	1	/* request to resolve address */
#define	ARPOP_REPLY	2	/* response to previous request */
#define	ARPOP_REVREQUEST 3	/* request protocol address given hardware */
#define	ARPOP_REVREPLY	4	/* response giving protocol address */
#define	ARPOP_INVREQUEST 8 	/* request to identify peer */
#define	ARPOP_INVREPLY	9	/* response identifying peer */
};
...



>How-To-Repeat:

# cd /usr/src/sys/arch/i386/compile/GENERIC/
# cc -fno-builtin-printf -fno-builtin-log -O2 \
 -pipe -Os -march=i686 \
 -nostdinc -I. -I../../../../arch -I../../../.. \
 -DDIAGNOSTIC -DKTRACE -DCRYPTO -DSYSVMSG -DSYSVSEM -DSYSVSHM \
 -DCOMPAT_35 -DCOMPAT_43 -DLKM -DFIFO -DINET -DI686_CPU -DDEBUG \
 -D_KERNEL -Di386 -c ../../../../netinet/if_ether.c
# make
# make install

reboot and arp will probably not work

for informaton
# gcc -v
Reading specs from /usr/lib/gcc-lib/i386-unknown-openbsd3.8/3.3.5/specs
Configured with: 
Thread model: single
gcc version 3.3.5 (propolice)


>Fix:
an easy fix to avoid this gcc bug is possible in function in_arpinput() replace
int op;
with
u_int16_t op;

but a short int to int assignment bug concerning at least gcc with
 -Os -march=i686

is in the optimization code of gcc 3.3.5 when
 TARGET_PENTIUMPRO is true and optimize_size is true
in i386.c  (/usr/src/gnu/usr.bin/gcc/gcc/config/i386/i386.c).


my related diff for the involved files (excluding gcc, but including
minor fixes for CARP and a more efficient htonl/ntohl for i486+
architectures in endian.h)
is below and here (yoy are reading it):
http://www.allerta.it/pub/improvements/openbsd/kernel/if_ether_plus_endian.diff

in the future you'll possibly find the gcc patch link here:
http://www.allerta.it/


All the best,
MARco
-- 
x(t),y(t) = th(3t-34.5)*e^[-(3t-34.5)^2]/2-4.3+e^(-1.8/t^2)/(.8*atg(t-
3)+2)(t-1.8)-.3th(5t-42.5),(1.4e^[-(3t-34.5)^2]+1-sgn[|t-8.5|-.5]*1.5*
|sin(pi*t)|^[2e^(-(t-11.5)^2)+.5+e^(-(.6t-3.3)^2)])/(.5+t)+1  ; 0<t<14

>Fix:
Index: if_ether.c
Author: Marco Munari da Castelfranco Veneto
(lines marked with + added and delta between lines marked with -/+)
===================================================================
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.61
diff -c -r1.61 if_ether.c
--- if_ether.c	2005/11/29 02:59:42	1.61
+++ if_ether.c	2006/02/19 02:44:30
@@ -77,7 +77,7 @@
 /* timer values */
 int	arpt_prune = (5*60*1);	/* walk list every 5 minutes */
 int	arpt_keep = (20*60);	/* once resolved, good for 20 more minutes */
-int	arpt_down = 20;		/* once declared down, don't send for 20 secs */
+int	arpt_down = 20;		/* once declared down, don't send for 20s */
 #define	rt_expire rt_rmx.rmx_expire
 
 void arptfree(struct llinfo_arp *);
@@ -352,6 +352,9 @@
 	bcopy((caddr_t)tip, (caddr_t)ea->arp_tpa, sizeof(ea->arp_tpa));
 	sa.sa_family = pseudo_AF_HDRCMPLT;
 	sa.sa_len = sizeof(sa);
+#ifdef DIAGNOSTIC
+	printf("arprequesth: %x%x%x%x%x",(int*)sa,*(int*)(&sa+4),*(int*)(&sa+8),*(int*)(&sa+12),*(int*)(&sa+16));
+#endif
 	(*ifp->if_output)(ifp, m, &sa, (struct rtentry *)0);
 }
 
@@ -528,12 +531,14 @@
 	struct sockaddr sa;
 	struct in_addr isaddr, itaddr, myaddr;
 	u_int8_t *enaddr = NULL;
-	int op;
+	u_int16_t op;
 
 	ea = mtod(m, struct ether_arp *);
 	op = ntohs(ea->arp_op);
-	if ((op != ARPOP_REQUEST) && (op != ARPOP_REPLY))
+	if ((op != ARPOP_REQUEST) && (op != ARPOP_REPLY)) {
+		printf("in_arpinput out: op=%x != ARPOP_ REQUEST/REPLY",op);
 		goto out;
+	}
 #if notyet
 	if ((op == ARPOP_REPLY) && (m->m_flags & (M_BCAST|M_MCAST))) {
 		log(LOG_ERR,
@@ -592,7 +597,11 @@
 		}
 	}
 
-	if (ia == NULL && m->m_pkthdr.rcvif->if_type != IFT_CARP) {
+	if (ia == NULL
+#if NCARP > 0
+	    && m->m_pkthdr.rcvif->if_type != IFT_CARP
+#endif
+	    ) {
 		struct ifaddr *ifa;
 
 		TAILQ_FOREACH(ifa, &m->m_pkthdr.rcvif->if_addrlist, ifa_list) {
@@ -696,9 +705,11 @@
 		if (la == 0)
 			goto out;
 		rt = la->la_rt;
+#if NCARP > 0
 		if (rt->rt_ifp->if_type == IFT_CARP &&
 		    m->m_pkthdr.rcvif->if_type != IFT_CARP)
 			goto out;
+#endif
 		bcopy(ea->arp_sha, ea->arp_tha, sizeof(ea->arp_sha));
 		sdl = SDL(rt->rt_gateway);
 		bcopy(LLADDR(sdl), ea->arp_sha, sizeof(ea->arp_sha));
@@ -815,13 +826,28 @@
 {
 	struct arphdr *ar;
 
-	if (m->m_len < sizeof(struct arphdr))
+	if (m->m_len < sizeof(struct arphdr)) {
+#ifdef DIAGNOSTIC
+		printf("revarpinput out: m_len=%d < (sizeof(struct arphdr)=)%d\n",
+		       m->m_len,sizeof(struct arphdr));
+#endif
 		goto out;
+	}
 	ar = mtod(m, struct arphdr *);
-	if (ntohs(ar->ar_hrd) != ARPHRD_ETHER)
+	if (ntohs(ar->ar_hrd) != ARPHRD_ETHER) {
+#ifdef DIAGNOSTIC
+		printf("revarpinput out: ntohs(ar->ar_hrd)=%d != ARPHRD_ETHER\n",
+		       ntohs(ar->ar_hrd),ARPHRD_ETHER);
+#endif
 		goto out;
-	if (m->m_len < sizeof(struct arphdr) + 2 * (ar->ar_hln + ar->ar_pln))
+	}
+	if (m->m_len < sizeof(struct arphdr) + 2 * (ar->ar_hln + ar->ar_pln)) {
+#ifdef DIAGNOSTIC
+		printf("revarpinput out: m_len=%d < (sizeof(struct arphdr)=)%d+2*((ar_hln=)%d+(ar_pln=)%d)\n",
+		       m->m_len,sizeof(struct arphdr),ar->ar_hln,ar->ar_pln);
+#endif
 		goto out;
+	}
 	switch (ntohs(ar->ar_pro)) {
 
 	case ETHERTYPE_IP:
@@ -830,6 +856,10 @@
 		return;
 
 	default:
+#ifdef DIAGNOSTIC
+		printf("revarpinput break: ntohs(ar->ar_pro) is 0x%x\n",
+		       ntohs(ar->ar_pro));
+#endif
 		break;
 	}
 out:


--- endian.h	14 Dec 2005 12:59:16 +0100	1.14
+++ endian.h	19 Feb 2006 04:57:46 +0100	
@@ -24,19 +24,161 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+/* 
+ * extra improvements
+ * copyright (c) 2005, 2006 by Marco Munari <mar.develops allerta.it>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of allerta nor the names of its contributors
+ *    may be used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * SOURCE CODE PROVIDED ``AS IS'' WITHOUT WARRANTIES, BUT WITH CARE.
+ *
+ */
+
+
 #ifndef _I386_ENDIAN_H_
 #define _I386_ENDIAN_H_
 
 #ifdef __GNUC__
 
-#if defined(_KERNEL) && !defined(I386_CPU)
+#if (defined(I486_CPU) || defined(I586_CPU) || defined(I686_CPU))
+
 #define	__swap32md(x) ({						\
 	u_int32_t __swap32md_x = (x);					\
 									\
 	__asm ("bswap %1" : "+r" (__swap32md_x));			\
 	__swap32md_x;							\
 })
+
+#define	__swap64md(x)	/* Marco Munari */				\
+	({								\
+		u_int64_t __swap64md_x = (x);				\
+		__asm (	"bswap %%eax\n"					\
+			"bswap %%edx\n"					\
+			"xchgl %%eax,%%edx"				\
+			: "+A" (__swap64md_x));			\
+		__swap64md_x;						\
+	})
+
+#define __reverse32md(x) /*MARco MunARi*/ ({			\
+	u_int32_t __reverse32md_x = (x);	\
+	__asm __volatile(					\
+		"	bswap	%1\n"				\
+		"	movb	$4, %%cl\n"			\
+								\
+		"	movl	%1, %%edx\n"			\
+		"	shrl	%%cl, %1\n"			\
+		"	andl	$0x0f0f0f0f, %1\n"		\
+		"	shll	%%cl, %%edx\n"			\
+		"	andl	$0xf0f0f0f0, %%edx\n"		\
+		"	orl	%%edx, %1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movl	%1, %%edx\n"			\
+		"	shrl	%%cl, %1\n"			\
+		"	andl	$0x33333333, %1\n"		\
+		"	shll	%%cl, %%edx\n"			\
+		"	andl	$0xcccccccc, %%edx\n"		\
+		"	orl	%%edx, %1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movl	%1, %%edx\n"			\
+		"	shrl	%%cl, %1\n"			\
+		"	andl	$0x55555555, %1\n"		\
+		"	shll	%%cl, %%edx\n"			\
+		"	andl	$0xaaaaaaaa, %%edx\n"		\
+		"	orl	%%edx, %1\n"			\
+		:"+abSD" (x)					\
+		:						\
+		:"%cl","%edx");					\
+	__reverse32md_x;					\
+})
+
+#define __reverse16md(x) /*MARco MunARi*/ ({			\
+	u_int16_t __reverse16md_x = (x);			\
+	__asm __volatile(					\
+		"	xchgb	%b1,%h1\n"			\
+		"	movb	$4, %%cl\n"			\
+								\
+		"	movw	%w1, %%dx\n"			\
+		"	shrw	%%cl, %1\n"			\
+		"	andw	$0x0f0f, %w1\n"			\
+		"	shlw	%%cl, %%dx\n"			\
+		"	andw	$0xf0f0, %%dx\n"		\
+		"	orw	%%dx, %w1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movw	%w1, %%dx\n"			\
+		"	shrw	%%cl, %w1\n"			\
+		"	andw	$0x3333, %w1\n"			\
+		"	shlw	%%cl, %%dx\n"			\
+		"	andw	$0xcccc, %%dx\n"		\
+		"	orw	%%dx, %w1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movw	%w1, %%dx\n"			\
+		"	shrw	%%cl, %w1\n"			\
+		"	andw	$0x5555, %w1\n"			\
+		"	shlw	%%cl, %%dx\n"			\
+		"	andw	$0xaaaa, %%dx\n"		\
+		"	orw	%%dx, %w1\n"			\
+		:"+ab" (x)					\
+		:						\
+		:"%cl","%dx");					\
+	__reverse16md_x;					\
+})
+
+#define __reverse8md(x) /*MARco MunARi*/ ({			\
+	u_int16_t __reverse8md_x = (x);				\
+	__asm __volatile(					\
+		"	movb	$4, %%cl\n"			\
+								\
+		"	movb	%b1, %h1\n"			\
+		"	shrb	%%cl, %1\n"			\
+		"	andb	$0x0f, %b1\n"			\
+		"	shlb	%%cl, %h1\n"			\
+		"	andb	$0xf0, %h1\n"			\
+		"	orb	%h1, %b1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movb	%b1, %h1\n"			\
+		"	shrb	%%cl, %1\n"			\
+		"	andb	$0x33, %b1\n"			\
+		"	shlb	%%cl, %h1\n"			\
+		"	andb	$0xcc, %h1\n"			\
+		"	orb	%h1, %b1\n"			\
+								\
+		"	shrb	$1, %%cl\n"			\
+								\
+		"	movb	%b1, %h1\n"			\
+		"	shrb	%%cl, %1\n"			\
+		"	andb	$0x55, %b1\n"			\
+		"	shlb	%%cl, %h1\n"			\
+		"	andb	$0xaa, %h1\n"			\
+		"	orb	%h1, %b1\n"			\
+								\
+		:"+abd" (x)					\
+		:						\
+		:"%cl");					\
+	__reverse8md_x;						\
+})
+
 #else
+
 #define	__swap32md(x) ({						\
 	u_int32_t __swap32md_x = (x);					\
 									\
@@ -44,20 +186,53 @@
 	    "+r" (__swap32md_x));					\
 	__swap32md_x;							\
 })
-#endif	/* _KERNEL && !I386_CPU */
 
 #define	__swap64md(x) ({						\
 	u_int64_t __swap64md_x = (x);					\
-									\
 	(u_int64_t)__swap32md(__swap64md_x >> 32) |			\
 	    (u_int64_t)__swap32md(__swap64md_x & 0xffffffff) << 32;	\
 })
+
+#endif	/* !I386_CPU */
+
+
+#if defined(I386_CPU)	/* xxx check CPU optimal pipelines skeduling condition */
 #define	__swap16md(x) ({						\
 	u_int16_t __swap16md_x = (x);					\
-									\
-	__asm ("rorw $8, %w1" : "+r" (__swap16md_x));			\
+	__asm ("xchgb %b1, %h1" : "+Q" (__swap16md_x));			\
 	__swap16md_x;							\
 })
+/* I386_CPU */
+#else  /* !I386_CPU */
+#define	__swap16md(x) ({						\
+	u_int16_t __swap16md_x = (x);					\
+	__asm ("rolw $8, %1" : "+r" (__swap16md_x));			\
+	__swap16md_x;							\
+})
+#endif  /* !I386_CPU */
+
+
+
+
+//funny try: __asm ("xchgb %b1, %w1"##"h" : "+r" (__swap16md_x));
+/*
+//interesting, with:
+	__asm ("xchgb %b1, %h1" : "+r" (__swap16md_x));
+/usr/src/sys/net/pf.c:1178: internal compiler error: Segmentation fault
+Please submit a full bug report,
+with preprocessed source if appropriate.
+See <URL:http://gcc.gnu.org/bugs.html> for instructions.
+{standard input}: Assembler messages:
+{standard input}:6527: Warning: end of file not at end of a line; newline inserted
+{standard input}:8005: Error: bad register name `%sil'
+
+another try:
+	__asm ("xchgl %1,%2"						\
+	       : "+r" (__swap32md((u_int32_t)__swap64md_x)),		\
+	       "+r" (__swap32md((u_int32_t)(__swap64md_x>>32))));	\
+	__swap64md_x;							\
+
+*/
 
 /* Tell sys/endian.h we have MD variants of the swap macros.  */
 #define MD_SWAP
