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: Sun, 21 Feb 2021 21:03:28 +0000 Message-ID: References: <87a6ry46uc.fsf@collares.org> Reply-To: Andrea Corallo Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="5318"; 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 Sun Feb 21 22:04: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 1lDvtl-0001Hf-F2 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 21 Feb 2021 22:04:13 +0100 Original-Received: from localhost ([::1]:34594 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lDvtk-0003KJ-Gu for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 21 Feb 2021 16:04:12 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:47872) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lDvtc-0003K7-6a for bug-gnu-emacs@gnu.org; Sun, 21 Feb 2021 16:04:05 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:43483) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lDvta-0001i5-Cz for bug-gnu-emacs@gnu.org; Sun, 21 Feb 2021 16:04:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lDvta-0000HZ-7s for bug-gnu-emacs@gnu.org; Sun, 21 Feb 2021 16:04:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Andrea Corallo Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 21 Feb 2021 21:04:02 +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.16139414191049 (code B ref 46670); Sun, 21 Feb 2021 21:04:02 +0000 Original-Received: (at 46670) by debbugs.gnu.org; 21 Feb 2021 21:03:39 +0000 Original-Received: from localhost ([127.0.0.1]:55029 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lDvtC-0000Gq-Lt for submit@debbugs.gnu.org; Sun, 21 Feb 2021 16:03:39 -0500 Original-Received: from mx.sdf.org ([205.166.94.24]:51275) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lDvtA-0000Ge-SU for 46670@debbugs.gnu.org; Sun, 21 Feb 2021 16:03:37 -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 11LL3SLc005583 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits) verified NO); Sun, 21 Feb 2021 21:03:30 GMT In-Reply-To: (Pip Cet's message of "Sun, 21 Feb 2021 11:56:08 +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:200527 Archived-At: Pip Cet writes: > On Sun, Feb 21, 2021 at 11:51 AM Pip Cet wrote: >> Does the attached patch help? Andrea, is my analysis correct? > > CCing Andrea. > > In more detail, some debugging showed we were trying to intersect a > "nil or t" constraint with a "sequence" constraint, the result being a > null constraint (t is not a sequence). So if (assume (and a b)) was > meant to imply the intersection of a and b, we're emitting it > incorrectly. Hi Pip, thanks for looking into this. 'and' is meant to imply intersection, so yeah... as you guess there's some problem in the LIMPLE we generate :) Just to mention we have also a number of tests in comp-tests.el to checks that we predict the correct return value, applying the suggested patch a number of these are not passing. We'll want to add also a new test there for this specific case when the issue is solved. Here a slightly more reduced test-case I'm using here for the analysis for which the compiler erroneously proves the return type is null. (let ((comp-verbose 3)) (native-compile '(=CE=BB (s) (and (equal (foo (length s)) s) s)))) Here is why, looking at LIMPLE coming in the final pass in bb_1 we emit: (assume #(mvar 41121096 0 null) (and #(mvar 42082902 0 sequence) #(mvar 411= 21566 1 boolean))) bb_1 is the basic block where 'equal' is verified so we want to enforce that the result cstr of the call 'foo' is intersected with 's' because they are equal. Now the trouble is that while 's' here is represented correctly by the mvar 42082902 the other operand of the and constraint is wrong. Where this is coming from then? Inside the predecessor block bb_0 we have the compare and branch sequence: (set #(mvar 41121566 1 boolean) (call equal #(mvar 42082358 1 t) #(mvar 416= 65638 2 sequence))) (cond-jump #(mvar 41121566 1 boolean) #(mvar nil nil null) bb_2_cstrs_0 bb_= 1) Here when we run the 'add-cstrs' pass `comp-add-cond-cstrs' is deciding that 42082358 41665638 must be constrained and therefore is emitting the assume. Unfortunatelly because 42082358 and 41121566 are sharing the same slot number when we do the next SSA rename the mvar referenced in the assume will be one that is produced by 'equal' and not the one that is consumed. The correct fix is to have `comp-add-cond-cstrs' rewrite the comparison sequence as something like: (set #(mvar nil X t) #(mvar 42082358 1 t)) (set #(mvar 41121566 1 boolean) (call equal #(mvar 42082358 1 t) #(mvar 416= 65638 2 sequence))) (cond-jump #(mvar 41121566 1 boolean) #(mvar nil nil null) bb_2_cstrs_0 bb_= 1) Where X is a new slot we add to the frame. We will reference this slot number in the assume instead of 1 so it does not get clobbered. Now I'm not 100% sure how trivial is to do that as no pass is ATM changing the frame size. I guess I'll try to write a patch tomorrow evening. Thanks! Andrea