From patchwork Wed Feb 1 18:19:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 9550523 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0856A60425 for ; Wed, 1 Feb 2017 18:21:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ECE7828433 for ; Wed, 1 Feb 2017 18:21:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E0C6928451; Wed, 1 Feb 2017 18:21:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_SORBS_SPAM,T_DKIM_INVALID autolearn=no version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7FDCE28433 for ; Wed, 1 Feb 2017 18:21:24 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cYzWx-0001V4-F8; Wed, 01 Feb 2017 18:21:19 +0000 Received: from mail-it0-f51.google.com ([209.85.214.51]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cYzWs-0001Pr-7X for linux-arm-kernel@lists.infradead.org; Wed, 01 Feb 2017 18:21:16 +0000 Received: by mail-it0-f51.google.com with SMTP id 203so22786303ith.0 for ; Wed, 01 Feb 2017 10:20:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=J4QvLb1PDuw09Jve0Y+/q6BMWb3wvZB6HpvzBYHpwbU=; b=cz0KlgHEFNmoD4UTrxhaCJEfSL2zKP1gZQHdGPhzmG1SWyH6uKmJdpXe9YMtqtMMnN bBmrw2x9/i/9lfds4IqkeG5wvvTHnoss1xK/PZ6hUTsnBkyepBWI/0sDEbdzH3RUuP06 hXYh6QWB58Gu8PFgOgx9yikho+/t31557UHco= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=J4QvLb1PDuw09Jve0Y+/q6BMWb3wvZB6HpvzBYHpwbU=; b=flisFahXUUszsktv3c5AP1SEmKiPn8eCXtMWye36+xK3XIfxwzwtHaZxKI0WnZNNQ+ r3ayIauy9byltMdEtptdFyTMOdQijKqkeIAYLQW7Kc0OwP9owXQ8usxMLX1KNhJEHnxO 0urpRRCJ27tSnoMv1bk0/6lBsCOSQu+NTfL2hZT0FtZUVbnzV0T+ciObwqhaK9UwueNq ItBjWIJ1r3PGFjwsgLAmv0YwLEr3U93eZjZ4GZktwsTpGqSS99CbUe7HF2Hm9TMfbeJY lGwp+kUqKg+IXzXCD7KmohwNiomSvQtiY7SCz4znieoWXg4iHPj3RGJdJQHeUz1VPweg uoVQ== X-Gm-Message-State: AIkVDXJHfofbGgofEJ2rMy23BUsAc2hk6/ZteAxCq93bu1d+gBMMFinQCIfoxHaX9umZqFs05vyPJGOrqqJfsnaE X-Received: by 10.36.77.10 with SMTP id l10mr3197751itb.59.1485973193059; Wed, 01 Feb 2017 10:19:53 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.144.139 with HTTP; Wed, 1 Feb 2017 10:19:52 -0800 (PST) In-Reply-To: References: <20161017183806.GG5601@arm.com> <20161019153746.GA4411@x4> <20161019155658.GB4411@x4> <20161019162222.GT9193@arm.com> From: Ard Biesheuvel Date: Wed, 1 Feb 2017 18:19:52 +0000 Message-ID: Subject: Re: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness To: Laura Abbott X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170201_102114_323444_871CAF36 X-CRM114-Status: GOOD ( 20.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Zijlstra , Stephen Boyd , Will Deacon , Linux Kernel Mailing List , james.greenhalgh@arm.com, Markus Trippelsdorf , Gregory Clement , Linus Torvalds , Ingo Molnar , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 1 February 2017 at 17:36, Ard Biesheuvel wrote: > On 1 February 2017 at 16:58, Laura Abbott wrote: >> On 10/19/2016 09:22 AM, Will Deacon wrote: >>> On Wed, Oct 19, 2016 at 09:01:33AM -0700, Linus Torvalds wrote: >>>> On Wed, Oct 19, 2016 at 8:56 AM, Markus Trippelsdorf >>>> wrote: >>>>> On 2016.10.19 at 08:55 -0700, Linus Torvalds wrote: >>>>>> >>>>>> Well, in the meantime we apparently have to live with it. Unless Will >>>>>> is using some unreleased gcc version that nobody else is using and we >>>>>> can just ignore it? >>>>> >>>>> Yes, he is using gcc-7 that is unreleased. (It will be released April >>>>> next year.) >>>> >>>> Ahh, self-built? So it's not part of some experimental ARM distro >>>> setup and this will be annoying lots of people? >>> >>> Our friendly compiler guys built it, but it's just a snapshot of trunk, >>> so it's all heading towards GCC 7.0. AFAIU, the problematic optimisation >>> is also a mid-end pass, so it would affect other architectures too. >>> >>>> If so, still think that we could just get rid of the ____ilog2_NaN() >>>> thing as it's not _that_ important, but it's certainly not very >>>> high-priority. Will can do it in his tree too for testing, and it can >>>> remind people to get the gcc problem fixed. >>> >>> I'm carrying the diff below, which fixes arm64 defconfig, but I'm worried >>> that we might be relying on this trick elsewhere. The arm __bad_cmpxchg >>> function, for example. >>> >>> Will >>> >>> --->8 >>> >>> diff --git a/include/linux/log2.h b/include/linux/log2.h >>> index fd7ff3d91e6a..9cf5ad69065d 100644 >>> --- a/include/linux/log2.h >>> +++ b/include/linux/log2.h >>> @@ -16,12 +16,6 @@ >>> #include >>> >>> /* >>> - * deal with unrepresentable constant logarithms >>> - */ >>> -extern __attribute__((const, noreturn)) >>> -int ____ilog2_NaN(void); >>> - >>> -/* >>> * non-constant log of base 2 calculators >>> * - the arch may override these in asm/bitops.h if they can be implemented >>> * more efficiently than using fls() and fls64() >>> @@ -85,7 +79,7 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >>> #define ilog2(n) \ >>> ( \ >>> __builtin_constant_p(n) ? ( \ >>> - (n) < 1 ? ____ilog2_NaN() : \ >>> + (n) < 1 ? 0 : \ >>> (n) & (1ULL << 63) ? 63 : \ >>> (n) & (1ULL << 62) ? 62 : \ >>> (n) & (1ULL << 61) ? 61 : \ >>> @@ -149,9 +143,7 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >>> (n) & (1ULL << 3) ? 3 : \ >>> (n) & (1ULL << 2) ? 2 : \ >>> (n) & (1ULL << 1) ? 1 : \ >>> - (n) & (1ULL << 0) ? 0 : \ >>> - ____ilog2_NaN() \ >>> - ) : \ >>> + 0) : \ >>> (sizeof(n) <= 4) ? \ >>> __ilog2_u32(n) : \ >>> __ilog2_u64(n) \ >>> @@ -194,7 +186,6 @@ unsigned long __rounddown_pow_of_two(unsigned long n) >>> * @n: parameter >>> * >>> * The first few values calculated by this routine: >>> - * ob2(0) = 0 >>> * ob2(1) = 0 >>> * ob2(2) = 1 >>> * ob2(3) = 2 >>> >> >> Reviving this thread as gcc 7 has now hit Fedora rawhide and has this >> same issue. I pulled in the above patch from Will as a temporary work >> around for building. It didn't look like there was consensus on a >> permanent solution though from the thread. >> > > I still think order_base_2() is broken, since it may invoke > roundup_pow_of_two() with an input value that is documented as > producing undefined output. I would argue that the below is the > correct fix. > > diff --git a/include/linux/log2.h b/include/linux/log2.h > index fd7ff3d91e6a..46523731bec0 100644 > --- a/include/linux/log2.h > +++ b/include/linux/log2.h > @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n) > * ... and so on. > */ > > -#define order_base_2(n) ilog2(roundup_pow_of_two(n)) > +static inline __attribute__((__const__)) > +unsigned long __order_base_2(unsigned long n) > +{ > + return n ? 1UL << fls_long(n - 1) : 1; > +} > + > +#define order_base_2(n) \ > +( \ > + __builtin_constant_p(n) ? ( \ > + ((n) < 2) ? (n) : \ > + ilog2((n) - 1) + 1) : \ > + ilog2(__order_base_2(n)) \ > + ) > > #endif /* _LINUX_LOG2_H */ Actually, there is a still a redundant shift/fls() in there, this is even simpler: diff --git a/include/linux/log2.h b/include/linux/log2.h index fd7ff3d91e6a..4741534bd7af 100644 --- a/include/linux/log2.h +++ b/include/linux/log2.h @@ -203,6 +203,18 @@ unsigned long __rounddown_pow_of_two(unsigned long n) * ... and so on. */ -#define order_base_2(n) ilog2(roundup_pow_of_two(n)) +static inline __attribute__((__const__)) +unsigned long __order_base_2(unsigned long n) +{ + return n > 1 ? ilog2(n - 1) + 1 : 0; +} + +#define order_base_2(n) \ +( \ + __builtin_constant_p(n) ? ( \ + ((n) < 2) ? (n) : \ + ilog2((n) - 1) + 1) : \ + __order_base_2(n) \ + ) #endif /* _LINUX_LOG2_H */