From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Pip Cet Newsgroups: gmane.emacs.bugs,gmane.comp.lib.gnulib.bugs Subject: bug#36370: 27.0.50; XFIXNAT called on negative numbers Date: Fri, 28 Jun 2019 21:07:37 +0000 Message-ID: References: <8979488.cRkkfcT1mV@omega> <11002295.LrvMqknVDZ@omega> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="68687"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 36370@debbugs.gnu.org, Paul Eggert , bug-gnulib@gnu.org To: Bruno Haible Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jun 28 23:09:35 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1hgy7j-000Hlq-3l for geb-bug-gnu-emacs@m.gmane.org; Fri, 28 Jun 2019 23:09:35 +0200 Original-Received: from localhost ([::1]:36352 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgy7i-0006m9-1v for geb-bug-gnu-emacs@m.gmane.org; Fri, 28 Jun 2019 17:09:34 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:54187) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hgy7E-0006kk-Kn for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 17:09:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hgy7C-0008Fm-DX for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 17:09:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:57801) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hgy7B-0008F0-Tc for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 17:09:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hgy7B-0003wc-MP for bug-gnu-emacs@gnu.org; Fri, 28 Jun 2019 17:09:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Pip Cet Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 28 Jun 2019 21:09:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 36370 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 36370-submit@debbugs.gnu.org id=B36370.156175610415106 (code B ref 36370); Fri, 28 Jun 2019 21:09:01 +0000 Original-Received: (at 36370) by debbugs.gnu.org; 28 Jun 2019 21:08:24 +0000 Original-Received: from localhost ([127.0.0.1]:43111 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgy6Z-0003vZ-Hy for submit@debbugs.gnu.org; Fri, 28 Jun 2019 17:08:24 -0400 Original-Received: from mail-ot1-f41.google.com ([209.85.210.41]:41157) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hgy6U-0003vF-Np for 36370@debbugs.gnu.org; Fri, 28 Jun 2019 17:08:20 -0400 Original-Received: by mail-ot1-f41.google.com with SMTP id o101so7059010ota.8 for <36370@debbugs.gnu.org>; Fri, 28 Jun 2019 14:08:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=HOrMmQEm9jWBfwD7c7FjdOiWInhNHTe8lvhrU/9EVgU=; b=KybW0/5QBUmutANtQyM5zPaNaH3Rr8Z/4/occ2wvvdABlo98fR6A7Aw+tJn3LlFgMo yG41dVCZ7kUjLZkXbEFRjjPdp5Trxb5RgwExQydRacHrRUem7LBJWLY30cSi5joy7Cb9 TalCe+CMjsEdWsmcCkioCjhsDr8NDQ5Dz/C2/gvGq4egCfrOnMrPawdFmWsNZILzdYki RCbTmye2YgglBa1wbm/Ki1/L9tkrSUbUQPn9/Wvp9vdAsFVRcZ2p3hmmOalfhvfGmmRK H+7Li0vIXJ3JhJAtLnH4e5w8kh083MILBYTVUH6zlrEh90Ob9ZxDd3AKBDWOPlmA6/Vu 4fJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=HOrMmQEm9jWBfwD7c7FjdOiWInhNHTe8lvhrU/9EVgU=; b=GpXFBZLgsH86/ICalhtDDo0gEPHqndwJMjy0xKML3KCE9F2nH37xlwGE+9oES7LvTV F88px1fS14WBRmGJ5cZgXP09rUr4ugSxAGRx7QR/P8Dw+J7+Azhq6/GSw1fjVXbsZK/8 /ihTn2uB28m1nMDpRnzArO3AuSvU3ycz7IhF1vqapomoHCppzE4tK8S9hp9+onnOqlg9 hXTusef5SWEGsLYd8qFIl4gbSO3dr315Ueq6VkGXPBQ2/wQ7kktFTPFS3sP+S1PsrZ44 bqhIOTBk5SpJbTM0pmVZ6JINlKdBTNlnYULDZc46XufvmhTsK3i2Rg9IE1HS2vI/mQWk 8F9w== X-Gm-Message-State: APjAAAVUmB9JqjtPD42JYbxUHHzHhgDRRLlakqHM97lCLXzrYPKkI5Ew jBDnK417Pa74SrIVe8LW89waZBbBCJxBXEsEE/c= X-Google-Smtp-Source: APXvYqyAsHNltupA2PFc2hVGyp0OfMUHOxYyMdj5P+nqnA9UmTBE3JfLzJSehtkHVXazT5i8b7aTeZJNA/9ZHLAemvo= X-Received: by 2002:a9d:3c5:: with SMTP id f63mr9475512otf.210.1561756093089; Fri, 28 Jun 2019 14:08:13 -0700 (PDT) In-Reply-To: <11002295.LrvMqknVDZ@omega> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:161773 gmane.comp.lib.gnulib.bugs:40586 Archived-At: On Fri, Jun 28, 2019 at 7:11 PM Bruno Haible wrote: > Pip Cet wrote: > > Sorry, can't reproduce that here. I'm sure the changes I need to make > > are obvious once I've found them, but can you let me know your gcc > > version? > > I reproduce this with GCC versions 5.4.0, 6.5.0, 7.4.0, 8.3.0, and 9.1.0. > 1. Take the files foo.c and bar.c from > > 2. Compile and disassemble: > gcc -O2 -m32 -flto foo.c bar.c -shared -o libfoo.so && objdump --disassemble > libfoo.so > Observe that f_assume pushes an immediate $0 argument on the stack for the > function call. > 3. Enable the second 'assume' definition instead of the first one. > 4. Compile and disassemble: > gcc -O2 -m32 -flto foo.c bar.c -shared -o libfoo.so && objdump --disassemble > libfoo.so > Observe that f_assume is now an alias of f_generic, that pushes a > computed value as argument on the stack for the function call (push %eax). Thanks. I have no idea what I was doing wrong, but I can confirm that this case indeed demonstrates a GCC weakness when there is a non-split eassume (A && B), where A and B are both assumable (in the sense that __builtin_constant_p(!A == !A). > > Sorry to be pedantic, but do you disagree that it is better in these > > cases, or in general? > > I disagree that it is better in general. > You're apparently setting out a high goal for the 'assume' macro: > (1) that the programmer may call it with an expression that involves > function calls, That's the status quo in Emacs and many other projects that have started believing the "an inline function is as fast as a macro" mantra*, assuming you include inline functions with "function calls". (* - as have I) > (2) that the generated code will never include these function calls, > because the generated code with the 'assume' invocation should be > optimized at least as well as the generated code without the > 'assume' invocation. I think it should be the rarest of exceptions for an assume() to result in slower code, yes. I believe that includes the case where functions marked inline aren't inlined, because of compiler options, for example. > I'm adding certain quality criteria: > - It is not "good" if a construct behaves unpredictably, that is, > if it hard to document precisely how it will behave. (*) Uhm, isn't the whole point of assume() to make the program behavior undefined in a certain case? int i = 1; assume(i == 0); will behave unpredictably. That's why we use the assume(), to speed up execution by providing the compiler with an expression which "may or may not be evaluated", and allowing it to do just anything if the expression evaluates to false. That's the documented API. If you want your program to behave predictably, in the strict sense, you cannot ever use the current assume() API. If you want it to behave predictably in some looser sense, the sense I suggest is that assume(C) may or may not evaluate C. Again, that's what the documentation says. > - It is not "good" if the behaviour with no LTO is different from > the behaviour with LTO. I agree I should investigate the LTO example further, but please let's keep in mind that I observed problematic behavior of assume(A && B) in other contexts, so I'm not convinced LTO is the decisive factor here. However, optimization can and will influence program behavior in many ways. One of these ways is whether the argument to assume, which the programmer knows "may or may not be evaluated", is evaluated or not. > The implementation with the __builtin_constant, while attaining the > goals (1) and (2), does not satisfy the two quality criteria. For the record, my quality criteria are: (1) implement the documented API, and don't change it (2) when optimizing for speed, do not produce slower code with eassume() than we would without it. Even when the programmer wrongly guessed that a function would be inlined. (3) when optimizing for size, do not produce larger code with eassume() than we would without it. Even when inline functions are not inlined. (4) be at least as fast as gnulib assume() > I believe the only way to attain the goals and the quality criteria > is, as you suggested, to ask the GCC people to add a __builtin_assume > built-in. I think there's a significant probability that the GCC people would agree to add such a built-in, but insist on its having "may or may not evaluate its argument" semantics. > > > 1. The new 'assume' is worse when -flto is in use. > > > > Maybe. Even if it is, though, that's a GCC limitation which I consider > > likely to be fixable > > Yes, *maybe* the GCC people can change the semantics of __builtin_constant_p > so that it is effectively computed at link-time, rather than when a single > compilation unit gets compiled. Or maybe not. I don't know... Hmm. My observations suggest that __builtin_constant_p is effectively computed at link-time; all I have to do is to split the assume() in your example. > > It's way too easy to do something like > > > > eassume(ptr->field >= 0 && f(ptr)); > > > > when what you mean is > > > > eassume(ptr->field >= 0); > > eassume(f(ptr)); > > I argue that it's unnatural if the two don't behave exactly the same. But you're arguing for a __builtin_assume which doesn't evaluate its argument, still? Because { i++; __builtin_assume(i == 0); __builtin_assume(infloop()); i++; } could validly be optimized to i = 1, while { i++; __builtin_assume(i == 0 && infloop()); i++; } could not be. Or did I misunderstand the semantics you suggested for __builtin_assume? > Like everyone expects that > x = foo ? yes : no; > is equivalent to > if (foo) x = yes; else x = no; It is. > And everyone expects that > if (A && B) { ... } > is equivalent to > if (A) if (B) { ... } It is. Sorry if I'm being a bit dense here.