From owner-svn-src-all@FreeBSD.ORG Wed Feb 29 07:44:58 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 9C3F1106566B; Wed, 29 Feb 2012 07:44:58 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail26.syd.optusnet.com.au (mail26.syd.optusnet.com.au [211.29.133.167]) by mx1.freebsd.org (Postfix) with ESMTP id 1C9288FC13; Wed, 29 Feb 2012 07:44:57 +0000 (UTC) Received: from c211-30-171-136.carlnfd1.nsw.optusnet.com.au (c211-30-171-136.carlnfd1.nsw.optusnet.com.au [211.30.171.136]) by mail26.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q1T7is0t032294 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 29 Feb 2012 18:44:55 +1100 Date: Wed, 29 Feb 2012 18:44:54 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Bruce Evans In-Reply-To: <20120229160522.W2514@besplex.bde.org> Message-ID: <20120229181608.S2887@besplex.bde.org> References: <201202281939.q1SJdtYr084858@svn.freebsd.org> <4F4D3F5D.70905@FreeBSD.org> <201202282222.26343.tijl@freebsd.org> <20120229160522.W2514@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@FreeBSD.org, Tijl Coosemans , src-committers@FreeBSD.org, Dimitry Andric , svn-src-all@FreeBSD.org Subject: Re: svn commit: r232266 - in head/sys: amd64/include i386/include pc98/include x86/include X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Feb 2012 07:44:58 -0000 On Wed, 29 Feb 2012, Bruce Evans wrote: > I cleaned this up a bit according to ideas in my previous mails, and > added a comment about the confusing use of __bswap64_const() (now > named __bswap64_gen()) in __bswap64_var(): A minor problem with only having a macro version for __bswap64() turned up: > % -#define __bswap16_const(_x) (__uint16_t)((_x) << 8 | (_x) >> 8) > % - > % -#define __bswap16(_x) \ > % - (__builtin_constant_p(_x) ? \ > % - __bswap16_const((__uint16_t)(_x)) : __bswap16_var(_x)) > ... > % +#define ___bswap16(x) (__uint16_t)((x) << 8 | (x) >> 8) > % +#define __bswap16(x) (___bswap16((__uint16_t)(x))) When x a non-volatile variable, gcc and clang produce the good code "rolw $8,x" for "x = __bswap16(x);" on short x. But when x a a volatile variable, gcc and clang produce fairly horrible code, with 2 loads of x corresponding to the 2 accesses to x. This is probably required by volatile semantics, and is a problem for all unsafe macros, especially when their name says that they are safe (oops). When __bswap16 is implemented as an inline function for the var case like it used to be, it only loads x once and there are no problems with volatile variables. Optimizing to "rolw $8,x" might still be possible iff x is not volatile, but load-modify-store is probably better anyway. So any macro version must use gcc features to be safe. The following seems to work: #define __bswap16(x) __extension__ ({ __uint16_t __x = x; (__uint16_t)(__x << 8 | __x >> 8); }) clang now produces "rolw $8,x" when x is volatile. This seems to violate volatile semantics. gcc produces load-rolw-store then. Both produce "rolw $8,x" when x is not volatile. Bruce