From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752394AbcFVKig (ORCPT ); Wed, 22 Jun 2016 06:38:36 -0400 Received: from mga02.intel.com ([134.134.136.20]:45374 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752070AbcFVKic convert rfc822-to-8bit (ORCPT ); Wed, 22 Jun 2016 06:38:32 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,509,1459839600"; d="scan'208";a="833068876" From: "Levy, Amir (Jer)" To: Arnd Bergmann , 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 Subject: RE: [PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug Thread-Topic: [PATCH v2] byteswap: try to avoid __builtin_constant_p gcc bug Thread-Index: AQHRy5vEIdMMBgekvESAGPwG4+LLDJ/09ZQAgAAacwCAADfXkA== Date: Wed, 22 Jun 2016 10:25:57 +0000 Message-ID: References: <1780465.XdtPJpi8Tt@wuerfel> <3864016.Cn7XXDBM38@wuerfel> In-Reply-To: <3864016.Cn7XXDBM38@wuerfel> Accept-Language: he-IL, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.184.70.11] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-06-22 Arnd Bergmann wrote: > 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/ixgb > > > e/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 There are more than 20 files that have the statement: case cpu_to_... Sparse complains about: case __builtin_bswap, not about __builtin_constant_p. Amir