From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752238AbcFVKPy (ORCPT ); Wed, 22 Jun 2016 06:15:54 -0400 Received: from mout.kundenserver.de ([212.227.126.135]:59901 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752129AbcFVKPs (ORCPT ); Wed, 22 Jun 2016 06:15:48 -0400 From: Arnd Bergmann To: Tomas Winkler Cc: Andrew Morton , mm-commits@vger.kernel.org, "linux-kernel@vger.kernel.org" , linux-scsi@vger.kernel.org, jpoimboe@redhat.com, Martin Jambor , "Martin K. Petersen" , James Bottomley , Denys Vlasenko , Thomas Graf , Peter Zijlstra , David Rientjes , Ingo Molnar , Himanshu Madhani , Dept-Eng QLA2xxx Upstream , Jan Hubicka , "Amir (Jer)" Subject: Re: [PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug Date: Wed, 22 Jun 2016 11:59:30 +0200 Message-ID: <3864016.Cn7XXDBM38@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <1780465.XdtPJpi8Tt@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:VBI++Ap+//h5mKkd6r8OZxPwxCa2HAiX0eU5QSiOelPi4xGMFeS KohjnUZb7O/EhHfBg2E3Gg2eJUvUPK4qS3vlPrGZmv1mX8iZhe7zmwdAGVqx3oJyiv7GaLd YXJDwERaqISHj2Mj7HCO/PPqMZZOfcyZn1bSV9FTDTjRuJCgkIVirWdh1axUDi6vgxuqilj ZFI1zpYGZxymKR0s8+IQw== X-UI-Out-Filterresults: notjunk:1;V01:K0:mSESdeqZLdA=:ORos4lXAWyHf4FGS3SeabP TcnAohEQMf2F3gf8CVjQkGzoMkaEEZjYi7ZUjA9hcvz6xDLktBJt+uKN9Gr84eH4yxRlnD19I ph7YXHeWEE2ElEQTvNoMXhVJsbwMFHQkltxxU8zW6tivSWCwdSq2KFzZTfjgBC3I5gQzH1uf4 d5q/gWAv0vdN7keGN23J+9C6aJSNqD1iNwwkpLQ6ePmGMVoswAwhOVc64ntsNEouDDUxwBiun w84WlSPX6k+88GFkW6Q7mHeIIqQSe+rrXRAq5bkqOs1qezUcYgKB/SyZr3dzdq4HismjuQ9Ko 4Yjx06TK09b0byQcvgsfE0MHFB2hGyeglmvcHvoslvybR106seqxeU2l1/U171olAj80vJiTl bCvSppaQ4TgAOTIQjRYWHqbkeMkkWjPfRNICyhGR2wnWx2S+KcfV4H0xPuxINzlXUTRs8r5hF lpDQQlrC+JqGwLfkP+TvqnHEi3wWmp9PSTDDwI0UeZRCLbU++33+weP67GKqzUSoCLHSnj77c 7Jyuf1ExVVDdJm47hZCk4lN1g87AX89tlhwEoqxmbBbfxEAMHK+Qg54sUS7D21le77NHLBrxv 46F0sFrufk5J5LKebfIK8nX3mjhDL88WO+HTgUuy7HT/e3Ko+u3sB1pk/kmM8/EWiKS2onpv8 pYX0GJtFsiilgbgEflOFRXBpAGVkTA0yaVP9QalK5jn+p7Md/8TTJZI5LvCTOk+0YsWTbLD6j PVj6PFv8RdEwt4CK Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, June 22, 2016 11:24:50 AM CEST Tomas Winkler wrote: > On Tue, Jun 21, 2016 at 12:02 PM, Tomas Winkler wrote: > > On Tue, May 3, 2016 at 9:36 AM, Arnd Bergmann wrote: > >> On Monday 02 May 2016 16:32:25 Andrew Morton wrote: > >> #ifdef __HAVE_BUILTIN_BSWAP32__ > >> -#define __swab32(x) __builtin_bswap32((__u32)(x)) > >> +#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) > >> #else > >> #define __swab32(x) \ > >> (__builtin_constant_p((__u32)(x)) ? \ > > > >> > > > > I wonder if this doesn't break switch statement that requires a > > constant expression, there few cases like this over the kernel. > > > > switch(val) { > > case cpu_to_le32(IXGBE_RXDADV_STAT_FCSTAT_FCPRSP): > > > > http://lxr.free-electrons.com/source/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c#L458 > > > > I'm asking because sparse and checkpatch doesn't agree on that ping > sparse issues > 'error: bad constant expression' > When changing to __constant_cpu_to_le32 sparse is happy but > checkpatch.ps is complaining > __constant_cpu_to_le32 should be cpu_to_le32 > That is an interesting problem, as both seem to be reasonable: sparse probably doesn't understand __builtin_constant_p() enough, while avoiding __constant_cpu_to_le32() is a good recommendation in general. How many instances of this do you see in the kernel? If ixgbe is the only one, I'd just move the byteswap up into the switch statement: switch (le32_to_cpu(val)) { case IXGBE_RXDADV_STAT_FCSTAT_FCPRSP: which may cost one or two cycles for the non-constant byteswap, but is also easier to read than the current code. Arnd