From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Bruno Haible Newsgroups: gmane.comp.lib.gnulib.bugs,gmane.emacs.devel Subject: Re: more macOS Clang assume warnings Date: Wed, 26 Aug 2020 02:01:34 +0200 Message-ID: <1637416.KCIPBWfl1D@omega> References: <5B1F57F1-C671-40D8-8272-2C90E42F2538@acm.org> <87c35fba-515b-18ca-598c-72a0baa94113@cs.ucla.edu> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37548"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: KMail/5.1.3 (Linux/4.4.0-186-generic; KDE/5.18.0; x86_64; ; ) Cc: Mattias =?ISO-8859-1?Q?Engdeg=E5rd?= , bug-gnulib@gnu.org, emacs-devel To: Paul Eggert Original-X-From: bug-gnulib-bounces+gnu-bug-gnulib=m.gmane-mx.org@gnu.org Wed Aug 26 02:01:49 2020 Return-path: Envelope-to: gnu-bug-gnulib@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1kAisv-0009fC-86 for gnu-bug-gnulib@m.gmane-mx.org; Wed, 26 Aug 2020 02:01:49 +0200 Original-Received: from localhost ([::1]:55824 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kAisu-0000LW-9y for gnu-bug-gnulib@m.gmane-mx.org; Tue, 25 Aug 2020 20:01:48 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49002) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kAiso-0000Iu-Qf; Tue, 25 Aug 2020 20:01:42 -0400 Original-Received: from mo4-p00-ob.smtp.rzone.de ([85.215.255.24]:12344) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kAisl-0007iZ-Rm; Tue, 25 Aug 2020 20:01:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1598400096; s=strato-dkim-0002; d=clisp.org; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=0OC9LS3AnGTvqYODaAKsDbJ/+WaKhFJJ6a6qtQzrkXM=; b=bm7TRlj++HdHtvfq7ZDd+wvCa4D9top6Y8rZgMzG21XLTGXER3xKZU+aRxsjVjWQGk CeBvCBQwvc6FT8nHEmYtat4/7DrGGmJM1Vi+1vAL0XUxuC0NwVxyFSRM9Jkh/Wu0rppq SXQ6qYJMo3+uUDEWm4a3Avwh/il4kTll/UhzKJrHrCyrz7/cSvy3/+p2XF9K+QEvtr8g MklJOBOIIWBDW/v9aXfijnZedMSVBwklYP660r4n3TL3uZDBDipCWoxZ8IXheDMMpsAs JcjSn3BH7HGrvutB6jkFhdWYLd9mHI2pOkLyMTcwxlbKb5EH4tfBGgSswQYTeGGSNm3i jDiw== X-RZG-AUTH: ":Ln4Re0+Ic/6oZXR1YgKryK8brlshOcZlIWs+iCP5vnk6shH+AHjwLuWOHqfyyPs=" X-RZG-CLASS-ID: mo00 Original-Received: from bruno.haible.de by smtp.strato.de (RZmta 46.10.7 DYNA|AUTH) with ESMTPSA id z05f0fw7Q01YSEQ (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve X9_62_prime256v1 with 256 ECDH bits, eq. 3072 bits RSA)) (Client did not present a certificate); Wed, 26 Aug 2020 02:01:34 +0200 (CEST) In-Reply-To: <87c35fba-515b-18ca-598c-72a0baa94113@cs.ucla.edu> Received-SPF: none client-ip=85.215.255.24; envelope-from=bruno@clisp.org; helo=mo4-p00-ob.smtp.rzone.de X-detected-operating-system: by eggs.gnu.org: First seen = 2020/08/25 20:01:36 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] X-Spam_score_int: -46 X-Spam_score: -4.7 X-Spam_bar: ---- X-Spam_report: (-4.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, NICE_REPLY_A=-2.602, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnulib-bounces+gnu-bug-gnulib=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnulib" Xref: news.gmane.io gmane.comp.lib.gnulib.bugs:42753 gmane.emacs.devel:254232 Archived-At: [CCing bug-gnulib.] Paul Eggert wrote in : Mattias Engdeg=E5rd wrote: > > The latest assume reformulation caused several warnings like the follow= ing when building with Clang: > >=20 > > ../../emacs/src/xwidget.h:171:72: warning: control reaches end of non-v= oid > > function [-Wreturn-type] > > INLINE struct xwidget *lookup_xwidget (Lisp_Object obj) { eassume (0); } > > = ^ > > ../../emacs/src/eval.c:1571:1: warning: function declared 'noreturn' sh= ould not > > return [-Winvalid-noreturn] > > } > > ^ > >=20 > > This is with Apple clang version 11.0.0 (clang-1100.0.33.17) but it loo= ks like the same applies to clang trunk. >=20 > Thanks for reporting that. I can reproduce the problem with Clang 9.0.1 o= n=20 > Fedora 31. For the program at the end of this message, 'clang -O2 -Wall'= =20 > incorrectly complains "warning: function declared 'noreturn' should not r= eturn=20 > [-Winvalid-noreturn]". The patch below fixes this. Thanks for the report. > Bruno, how about if we go back to how verify.h did 'assume' before the re= cent=20 > clang-related changes? That would avoid the complaints. It might cost us = a few=20 > nanoseconds of performance with clang-generated code but that's no big de= al. Since the patch below fixes it without giving up on the optimizations for c= lang <=3D 8, I see no reason to give up on the approach with __builtin_assume. > And anyway, clang ought to get fixed to generate the slightly-better code= with=20 > 'verify.h' just as it was. It has already been fixed: clang >=3D 9 optimizes well with just __builtin_unreachable and no use of __builtin_assume. 2020-08-25 Bruno Haible verify: Avoid warnings when assume(0) is used. Reported by Mattias Engdeg=E5rd via Paul Eggert in . * lib/verify.h (assume): Use __builtin_unreachable if the argument is the constant 0. * tests/test-verify.c (f): New function. (state): New type. (test_assume_expressions, test_assume_optimization, test_assume_noreturn): New functions. diff --git a/lib/verify.h b/lib/verify.h index 6d7b961..ca2a154 100644 =2D-- a/lib/verify.h +++ b/lib/verify.h @@ -320,7 +320,9 @@ template based on __builtin_unreachable does not. (GCC so far has only __builtin_unreachable.) */ #if _GL_HAS_BUILTIN_ASSUME =2D/* Use a temporary variable, to avoid a clang warning +/* Use __builtin_constant_p to help clang's data-flow analysis for the case + assume (0). + Use a temporary variable, to avoid a clang warning "the argument to '__builtin_assume' has side effects that will be disca= rded" if R contains invocations of functions not marked as 'const'. The type of the temporary variable can't be __typeof__ (R), because that @@ -328,12 +330,16 @@ template instead. */ # if defined __cplusplus # define assume(R) \ =2D ((void) ({ bool _gl_verify_temp =3D (R); \ =2D __builtin_assume (_gl_verify_temp); })) + (__builtin_constant_p (R) && !(R) \ + ? (void) __builtin_unreachable () \ + : (void) ({ bool _gl_verify_temp =3D (R); \ + __builtin_assume (_gl_verify_temp); })) # else # define assume(R) \ =2D ((void) ({ _Bool _gl_verify_temp =3D (R); \ =2D __builtin_assume (_gl_verify_temp); })) + (__builtin_constant_p (R) && !(R) \ + ? (void) __builtin_unreachable () \ + : (void) ({ _Bool _gl_verify_temp =3D (R); \ + __builtin_assume (_gl_verify_temp); })) # endif #elif _GL_HAS_BUILTIN_UNREACHABLE # define assume(R) ((R) ? (void) 0 : __builtin_unreachable ()) diff --git a/tests/test-verify.c b/tests/test-verify.c index 498dd2f..7201ca1 100644 =2D-- a/tests/test-verify.c +++ b/tests/test-verify.c @@ -25,6 +25,8 @@ # define EXP_FAIL 0 #endif =20 +/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D T= est verify, verify_expr =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D */ + int x; enum { a, b, c }; =20 @@ -67,3 +69,43 @@ main (void) { return !(function (0) =3D=3D 0 && function (1) =3D=3D 8); } + +/* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D Test assume =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D */ + +static int +f (int a) +{ + return a; +} + +typedef struct { unsigned int context : 4; unsigned int halt : 1; } state; + +void +test_assume_expressions (state *s) +{ + /* Check that 'assume' accepts a function call, even of a non-const + function. */ + assume (f (1)); + /* Check that 'assume' accepts a bit-field expression. */ + assume (s->halt); +} + +int +test_assume_optimization (int x) +{ + /* Check that the compiler uses 'assume' for optimization. + This function, when compiled with optimization, should have code + equivalent to + return x + 3; + Use 'objdump --disassemble test-verify.o' to verify this. */ + assume (x >=3D 4); + return (x > 1 ? x + 3 : 2 * x + 10); +} + +_Noreturn void +test_assume_noreturn (void) +{ + /* Check that the compiler's data-flow analysis recognizes 'assume (0)'. + This function should not elicit a warning. */ + assume (0); +}