From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Pip Cet Newsgroups: gmane.emacs.bugs Subject: bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode Date: Thu, 25 Feb 2021 20:59:41 +0000 Message-ID: References: <87a6ry46uc.fsf@collares.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="20363"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 46670@debbugs.gnu.org, Mauricio Collares To: Andrea Corallo Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Feb 25 22:01:16 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1lFNl5-00058H-67 for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 25 Feb 2021 22:01:15 +0100 Original-Received: from localhost ([::1]:43954 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lFNl4-0000dK-38 for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 25 Feb 2021 16:01:14 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53452) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lFNku-0000d9-0M for bug-gnu-emacs@gnu.org; Thu, 25 Feb 2021 16:01:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:56154) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lFNkr-0003M2-Ru for bug-gnu-emacs@gnu.org; Thu, 25 Feb 2021 16:01:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lFNkr-0006gX-Q2 for bug-gnu-emacs@gnu.org; Thu, 25 Feb 2021 16:01:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Pip Cet Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 25 Feb 2021 21:01:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 46670 X-GNU-PR-Package: emacs Original-Received: via spool by 46670-submit@debbugs.gnu.org id=B46670.161428682825650 (code B ref 46670); Thu, 25 Feb 2021 21:01:01 +0000 Original-Received: (at 46670) by debbugs.gnu.org; 25 Feb 2021 21:00:28 +0000 Original-Received: from localhost ([127.0.0.1]:39467 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lFNkJ-0006fd-KR for submit@debbugs.gnu.org; Thu, 25 Feb 2021 16:00:28 -0500 Original-Received: from mail-ot1-f42.google.com ([209.85.210.42]:44125) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lFNkG-0006fP-Hc for 46670@debbugs.gnu.org; Thu, 25 Feb 2021 16:00:26 -0500 Original-Received: by mail-ot1-f42.google.com with SMTP id f33so7052312otf.11 for <46670@debbugs.gnu.org>; Thu, 25 Feb 2021 13:00:24 -0800 (PST) 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=T985JxnqMaBIur/aYjFSdn6qcsVftkPaEtbEiRV6WPI=; b=YOeygGWBmIObEMHrGbMkw4DWO4+E9/fpNfaAzngPN6YiIG39Oe5UCjc8f9/ozqFbkm iv2G56WDArIYFA6NuNVJJKVrTJi3LYSxP7gJcoZzcejMG8gEpgJZvnTLyeiNsgnrdTEw xcr9jLGJyaf/ChzRenCPYzPK1erIJ0EciZUmCiMJFciSrx+0oAy7OhQyHkt6WgLmf+Er ogKNloNoRvIUHmKrbBJO4E87BPUKWYSSXi6AELfR7hQxcaZEWnYi9qmVfqqjRA1FMaZt QXXvTCvD6PSOF4Vtr4ttTUqHrfxtMjWYHCQHH6HCEeVphCtIG9ov0qK7JiRF7rcEPu/F SKQg== 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=T985JxnqMaBIur/aYjFSdn6qcsVftkPaEtbEiRV6WPI=; b=hM4OvH06FGiW0SulH5a+ifMxGkJLQLcdjga6BBnb5VvaC+tDSYDPXHaFFYrHAHHCf/ Usx0l/Eg/xBp8in2SxtPlpxdtDlPwBwobPlDFPHIUB+VyXSEj1eCpNBN2cnoM4l9Sh4M yuk7OlTa4hXNFr0EW2gCyh9GbtiiQx1iH6YtFZ0DTmi8niLRzAuBmQajhKZ5WsTPA2qx vQnaR+7X7mNDPkOwXExt/NyuMnfJJSGCMsJbCdqIjYOAsc1Mr5sIboUaYjpEm/tK4YzD nyLWWtKGQOzYtPI3UotRehoYknfJnkaUg9IHE/5zOsfrIwobJDNhc9NTo+ew2Acvayn9 OuJw== X-Gm-Message-State: AOAM533oDIkoRVPclK1Jas1A3rS0wqC/4f+S/PDbJdCO+pGekbiMG0PF oq2YFmPgfrofKZ8l6EH8Qt+kYWhhUSF2hjRrsNI= X-Google-Smtp-Source: ABdhPJzuPMcAFC9aZHmC3sCAWqBudRv39T6lAfewd11JyiP47PxIQGyM0ZRC/Lnl3KDrSxbAwTwTtMfZKePMOSN5vn8= X-Received: by 2002:a05:6830:3090:: with SMTP id f16mr3860578ots.292.1614286818744; Thu, 25 Feb 2021 13:00:18 -0800 (PST) In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:200817 Archived-At: On Thu, Feb 25, 2021 at 4:56 PM Andrea Corallo wrote: > Pip Cet writes: > > On Wed, Feb 24, 2021 at 10:06 AM Andrea Corallo wrote: > > I thought I'd give this some time to let tempers cool. > > > > I appreciate your criticism (but not the ad hominem), and I will take > > it into account when communicating with you. I'd like to repeat that point. I do agree with some of the points you raise. > > As for the immediate issue: LIMPLE, of course, is yours to define as > > you wish. If, however, you don't define it, either in documentation or > > by providing code that handles it correctly, you can hardly blame me > > for considering it a bug if the obvious interpretation causes subtle, > > unnecessary problems. Again, that's three conditions that need to happen simultaneously: - no documentation - no code - behavior that contradicts the obvious interpretation In the case of `assume', all three are met: there's no documentation describing it, there's (currently) no code that uses assumes in a non-trivial fashion, and it doesn't mean "assume". > > To pick an example at random, after your recent changes, I assumed the > > following would be valid LIMPLE (pseudocode): > > > > (set (mvar :slot -1) (mvar :slot 0)) > > > > but it's not, because negatively-indexed "temporary variables" aren't > > actually mapped to variables in the C backend (instead, they generate > > out-of-bounds array accesses, usually a SIGSEGV). > > > > If that is intentional, we need to document it (we also need to assert > > rather than segfault when someone disregards this capricious rule) . > > If it is unintentional, and I believe it is > > Quoting myself this thread: > > "The best option is to decide that negative slot numbers are not rendered > into libgccjit IR and we consider these virtual to solve these kind of > cases." And then, much later, you were no longer talking about "virtual" negative slot numbers, you were talking about temporary variables (not metavariables) being kept live, and being created for any purpose. Mixed signals, at best. > I agree this should be documented. ATM LIMPLE is assumed to be correct > but an assertion in the back-end is a good idea. (At some point in the future, the backend needs to reject (not eassert, but error) invalid LIMPLE rather than crashing the Emacs process. I understand we're not there yet.) > > Lastly, on the subject of testing, I believe compiler correctness is > > fundamentally more important than not missing any optimizations. Most > > of your tests cover the latter aspect. Maybe we could express that in > > the tests somehow, by using another tag? > > I'd like to state a concept that I think is very pertinent here and most > likely overlooked: The difference between propagation related bugs and > correctness bugs is very thin if *not* existent at all. Many choices of > the compiler depends on what the propagation manage to proves. I'm not sure what you're saying or how it relates to what I said. > Misscompilations like exactly this bug (46670) are a perfect example of. Of what? This is clearly a correctness bug, not a missed optimization. If fixing it (and actually fixing the underlying issue, which I believe we haven't done yet) involves temporarily disabling some optimizations, then that's what we need to do. We can restore the optimizations later. > That said, ATM in comp-tests.el we have ~150 tests of which ~60 > verifying the value/type propagation through return type. (Yes. That doesn't contradict anything I said.) > On this subject I highly recommend the following, let's adopt what we > essentially do for GCC development: You might want to suggest that on emacs-devel, as it would be a very drastic change. If you, as an individual, want to stop responding to bug reports that don't meet your strict criteria, I don't think there's anyone trying to stop you. Just define a nice keyboard macro saying "I'm sorry, you need to jump through these hoops first". > If this rules are not followed is just too difficult and, above all, > expensive from a time prespective to review. This way of proceeding is > just the only sustainable. I try to avoid speaking in absolutes like that. There's more than one way to do it. > We probably should document these points somewhere. We should probably discuss and agree on them first. > >> Indeed I'm happy to answer as I've explained what this means in my > >> previous mail. > > > > I'm not sure I understand that sentence. I'll try looking through > > previous messages from you for an explanation, I guess? > > In my previous message I've explained how that assume works and what's > his meaning. I don't think so. We're looking at (assume mvar1 (not mvar2)) where the two mvars have overlapping lifetimes and share the same slot. That specializes to a contradiction. > >> And I've to say, not everything that's not working as you'd expect at > >> first glance is broken by definition, this is just not a very good > >> collaborative approach :/ > > > > My expectations are based, in part, on having read through your code, > > including the comments. That's hardly "at first glance". > > Unfortunatelly I understant that ATM reading the code and comments is > not sufficient to understand all mechanism and assumptions involved. Of course not. I didn't claim otherwise. > This is certainly in part because the compiler is just young It is, which is why assuming it's impeccably correct is not the productive approach right now. > That said FWIW my experience with Emacs and other big projects like GCC > is that debugging and experimentation is typically needed to get into > the deepness of, the expectation that reading code and doc is sufficient > is IME at least often just a chimera. And I didn't say it was sufficient. > Say I write a patch based on some assumption I'm convinced of and this > introduces a number of regressions. I did: the assumption I'm convinced of is that an assume translates into a test at runtime that must be true. > I'd either pick the simplest non > passing testcase and debug it to see what I've been missing I did. The regressions were optimizations that (with an exception, which you fixed) produced correct results from what I'm still convinced are incorrect intermediate representations. > or I'd ask > if the assumption I used is correct. I did. Your response can be summarized as "the tests are passing so there's no bug". > I'd not claim the compiler is > broken in some of its parts to begin with. After considering the alternatives, that still seems overwhelmingly likely to me. > Indeed as I've said will be my pleasure to answer any question if asked, > and even more to welcome any patch that improves the documentation as > outcome of this process. What, if anything, does "(assume mvar1 (not mvar2))" mean? > > Once we've done that, we can discuss why I think that > > would be a very bad design choice. > > I hope you don't think something you state is likely not to be fully > unrestood for now by you is already considered a very bad design choice. Of course I don't. > Please lets start from the assumtion that, to begin with, it works. I find "there is no bug" to be an unhelpful axiom when looking for a bug. Pip