From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751817AbcFWJ2g (ORCPT ); Thu, 23 Jun 2016 05:28:36 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:49766 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751460AbcFWJ2b (ORCPT ); Thu, 23 Jun 2016 05:28:31 -0400 From: Arnd Bergmann To: Tomas Winkler Cc: "Levy, Amir (Jer)" , 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 Date: Thu, 23 Jun 2016 11:29:18 +0200 Message-ID: <3921705.19HEvQgj2d@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> <7892604.CjmUVh55Wd@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:Ww0AnVzjim6NjuqN0oI1JJEr3Y/yJmeq90g1KinNgSlenWFyasL zsKzVYrDKxJsmSz5A9hC5pWaw1y+r4qKq2ntX1ui+l4+lb7tDDqx3SbCxz5KOYsxrn5MqxP UDluTYOQSkW66N/ygylu9a4jOnBynLuRj6Z8WTh3xzE9JPPb/6HOjarE+arPTvU26ydeWp7 DYv7aP9P/UrAdc/ZilRlA== X-UI-Out-Filterresults: notjunk:1;V01:K0:6usO/ssXzcU=:wMFfP/ltMcVgGi2d09w4u5 tB0UwoyHdqU73MfCVkgIvcfA/px5XsNCB+HVWzgUIRQt310kUpVkcMyOQe0mb1h959g/O2gXK zgHOpxYMEZgYosQXmEXG9oi9Y12ig28OzojPBhMYIB5OIUc6rPCongI4vDq2PqniqyAkzrTEB vObcjWMmlOZ2os8g1q3DZkwgikzmhX9dYOarDfh+87n0TEYaigevKg3eq0faw2t7HC8aI5Km3 Aj6mzWOSdF5AqDVISBSH9Q+LC2ag1xDg8YOrcbTji0xJ3JPtTYqDQa4r6K9BugdX35edrXKP+ v3VGwkCKnWBMnzC/KHgCTvaY1DGef4M1u1NnzZ+EbVcG084Jy6MULHLagoLOftqkSCRYPeSIu LCALbrWMgTANI+0UXzQDOFPo3uI6HIlvzZXktjyOr+Y+9VAxYDHU7m0g8Q/qH95Is0+WdD7Z0 Zi6wJNddoaFk0P3mcH5oLIdPDKikhCzIgoRh+0WIEby9pL0xSgPlbsdEyYbe546oJuAD/E+6B SgT7HMWqrA6TFhtnFjW+tPh9qa9ZsPqbzXIR+JQF46N0TRDat4+z3H/5p5ASDNsHp0fYb2/2V 2TOP2sBgzhxBI942fuMUj9DEMBPya2NEs2zl3MFheyiuh4+tVxuZMkHQ9dixNr9xV3c5l8eoo rseagcYzMxr0TfbQovhqPOKLjgnMuaDWZD3kuYeH5t5HhoRxrVD8lYygHmCcIeUqEdpxHXIIT ZvHj7O+Xnsdc9jVA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday, June 23, 2016 9:27:30 AM CEST Tomas Winkler wrote: > On Wed, Jun 22, 2016 at 3:25 PM, Arnd Bergmann wrote: > > On Wednesday, June 22, 2016 2:44:21 PM CEST Tomas Winkler wrote: > >> > > >> > There are more than 20 files that have the statement: case cpu_to_... > >> > Sparse complains about: case __builtin_bswap, not about __builtin_constant_p. > >> > >> There is even much more in the header files used in initializers, > >> which also require constants. I wonder if __builtin_bswap produces > >> constant expression correctly under gcc? > > > > In gcc-4.8 or later yes. gcc-4.6/4.7 on powerpc was a special case that we > > have worked around now, as the 16-bit byteswap there was not a constant > > expression, unlike the 32-bit and 64-bit ones. > > Can you please give any references to that? The patch description for the last change has a good explanation: commit 8634de6d254462e6953b7dac772a1df4f44c8030 Author: Josh Poimboeuf Date: Fri May 6 09:22:25 2016 -0500 compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() gcc support for __builtin_bswap16() was supposedly added for powerpc in gcc 4.6, and was then later added for other architectures in gcc 4.8. However, Stephen Rothwell reported that attempting to use it on powerpc in gcc 4.6 fails with: lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') ... I'm not entirely sure what those errors mean, but I don't see them on gcc 4.8. So let's consider gcc 4.8 to be the official starting point for __builtin_bswap16(). Arnd Bergmann adds: "I found the commit in gcc-4.8 that replaced the powerpc-specific implementation of __builtin_bswap16 with an architecture-independent one. Apparently the powerpc version (gcc-4.6 and 4.7) just mapped to the lhbrx/sthbrx instructions, so it ended up not being a constant, though the intent of the patch was mainly to add support for the builtin to x86: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624 has the patch that went into gcc-4.8 and more information." Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") Reported-by: Stephen Rothwell Tested-by: Stephen Rothwell Acked-by: Arnd Bergmann Signed-off-by: Josh Poimboeuf Signed-off-by: Stephen Rothwell Signed-off-by: Linus Torvalds include/linux/compiler-gcc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) > Just to make sure we are on the same line , the bottom line is that > sparse has to be adjusted. Yes, that sounds right. I don't know if sparse actually tracks the value of the constant, or if it's good enough for sparse to know that a constant input to __builtin_bswap results in a constant output. If that is the case, we could just #define __builtin_bswap32(x) (x) in the kernel headers when building with sparse, and have it work for older sparse versions too. Arnd