From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Andrea Corallo via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode Date: Tue, 23 Feb 2021 22:55:06 +0000 Message-ID: References: <87a6ry46uc.fsf@collares.org> Reply-To: Andrea Corallo Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37844"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 46670@debbugs.gnu.org, Mauricio Collares To: Pip Cet Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Feb 23 23:56:13 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 1lEgbC-0009f5-H1 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 23 Feb 2021 23:56:10 +0100 Original-Received: from localhost ([::1]:43868 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lEgbB-0005Dv-Hk for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 23 Feb 2021 17:56:09 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:48318) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lEgb3-0005Bo-SL for bug-gnu-emacs@gnu.org; Tue, 23 Feb 2021 17:56:01 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:49330) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lEgb3-0002vW-JO for bug-gnu-emacs@gnu.org; Tue, 23 Feb 2021 17:56:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lEgb3-00036A-HR for bug-gnu-emacs@gnu.org; Tue, 23 Feb 2021 17:56:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Andrea Corallo Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 23 Feb 2021 22:56: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.161412091411854 (code B ref 46670); Tue, 23 Feb 2021 22:56:01 +0000 Original-Received: (at 46670) by debbugs.gnu.org; 23 Feb 2021 22:55:14 +0000 Original-Received: from localhost ([127.0.0.1]:60876 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lEgaH-000358-Q6 for submit@debbugs.gnu.org; Tue, 23 Feb 2021 17:55:14 -0500 Original-Received: from mx.sdf.org ([205.166.94.24]:56610) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lEgaF-00034x-Eg for 46670@debbugs.gnu.org; Tue, 23 Feb 2021 17:55:12 -0500 Original-Received: from mab (ma.sdf.org [205.166.94.33]) by mx.sdf.org (8.15.2/8.14.5) with ESMTPS id 11NMt6tX013471 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits) verified NO); Tue, 23 Feb 2021 22:55:06 GMT In-Reply-To: (Pip Cet's message of "Tue, 23 Feb 2021 09:07:26 +0000") 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:200686 Archived-At: Pip Cet writes: > On Mon, Feb 22, 2021 at 11:16 AM Andrea Corallo wrote: >> Pip Cet writes: >> >> Yes but in this case (and probably others) we *have* to emit this >> >> assumption. >> > >> > Why? Are you saying the compiler requires (assume ...) insns for >> > correctness? If it does, I'm afraid that's a serious issue. >> >> It requires that assume if we want to infer precisely here. If we >> give-up emitting this assume it will just works perfectly but we'll miss >> to predict the return value as we should. > > Emitting an assumption about a dead variable only makes sense if the > dead variable matches a live one. And in that case, we can just emit > the assumption about the live variable to begin with. > >> > Again, that does seem very complicated, and if it improves >> > optimization we could probably improve it much more by modifying the >> > byte compiler to pop arguments in the caller rather than the callee. >> >> To me this sounds considerably more invasive, probably is because I'm >> much more used to work with the native compiler code that with the byte >> compiler one :) > > That is a little more invasive, but it's where you're going if you > make the major change of allocating your own stack slots rather than > letting the byte compiler do it. > >> I like the idea of the native compiler patch being less invasive as >> possible, this was the line I tried to follow and I think so far it >> paid. I guess a number of readers would'd agree with this kind of >> approach to begin with. > > I agree with it! That's why "leave slot allocation to the byte > compiler" is something I don't want you to throw away unnecessarily, > because it's a great simplifying assumption. > >> I think I should be able to work it out as discussed in one two evenings >> and it might be useful for other cases in the future too, so it does not >> sound as a big deal to me. > > It does to me, I must say. There's nothing special about comparisons: > you're turning a = a OP b two-operand code into c = a OP b > three-operand code. Your code won't be as good as genuine > three-operand code, and it won't be as simple as two-operand code. > >> >> > In fact, all we need to do is tell comp-cond-cstrs-target-mvar to >> >> > return nil more often, isn't it? >> >> >> >> Nope, the target mvar identified is the correct one, we just have ATM no >> >> way to reference it reliably into the assume. >> > >> > We don't have to, let's just not emit an assume about a variable that >> > we just introduced and that's never read? >> >> We disagree :) > > We can disagree about the "let's" part (and should, of course), but we > should agree about the "we don't have to" part, because that's a > matter of fact, not opinion. This new mvar *is* used, actually by an assume. And the assume in discussion is targetting a live variable that will be used by functional code so I really see no scandal here. >> As the byte compiler does not care about propagating types and values it >> can just clobber the variable, here we aim for more so we need it to >> keep it live till the assume. > > If we decide the variable needs to be kept live, the byte compiler > should keep it live, not us. Again no, any pass in the compiler must have the right to create new temporaries for whichever purpose it wants, and indeed we can't expect the byte-compiler to know in advance all passes in the native compiler and their scopes and goals. `add-cstrs' is just the first pass that now happen to have this need, is not a special case of any sort. Not giving the possibilities to passes to create variables would be totally equivalent to prevent GIMPLE passes in GCC to create new local variables while performing transformations, it would be ludicrous and this is just something we *want* to be able to do. Andrea