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: Mon, 22 Feb 2021 10:04: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="3723"; 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 Mon Feb 22 11:07:37 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 1lE87t-0000rx-Dx for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 22 Feb 2021 11:07:37 +0100 Original-Received: from localhost ([::1]:36010 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lE87s-0006gW-GZ for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 22 Feb 2021 05:07:36 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:45920) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lE86M-0005UR-CL for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 05:06:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:44147) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lE86M-0000n8-4w for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 05:06:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lE86L-0006YC-VY for bug-gnu-emacs@gnu.org; Mon, 22 Feb 2021 05:06: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: Mon, 22 Feb 2021 10:06: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.161398832425129 (code B ref 46670); Mon, 22 Feb 2021 10:06:01 +0000 Original-Received: (at 46670) by debbugs.gnu.org; 22 Feb 2021 10:05:24 +0000 Original-Received: from localhost ([127.0.0.1]:55693 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lE85k-0006XE-8m for submit@debbugs.gnu.org; Mon, 22 Feb 2021 05:05:24 -0500 Original-Received: from mail-ot1-f42.google.com ([209.85.210.42]:40560) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lE85i-0006Wo-QT for 46670@debbugs.gnu.org; Mon, 22 Feb 2021 05:05:23 -0500 Original-Received: by mail-ot1-f42.google.com with SMTP id b8so11418141oti.7 for <46670@debbugs.gnu.org>; Mon, 22 Feb 2021 02:05:22 -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=AFyySltaa8V9tju+y0URnlEXavXIT/655dGGhNpum2I=; b=C/lGt+/wc0FQzr+rvl0vu3h8+jcIEry+YLmlYnCL9qH07lazXQkIb2VPnXeLhORhcS ALqKEKoQb8kPmH01I0bNJsnPVJWWxkjeZc2BchcUfMl26JQ3gmj3BPU5i4SJgAmIpxJ7 0lSDy5ekx2xMXSrfwjPvhvZX0nsAO63t3v0J0R1IEKKea4IyQ5xXWov7E76jzIaR5c0a f5VLT8Gd+4PDzxlenJJjIokso16yn1MKDyrZnY1PcauzAbZsS7XGSqOWU2ijmIjAIc8S OEfDV9wOz1VjWXpiwQcgWNbBkvNWswG/KNfV7a3F0v1uPXu5oKg9o9429acSZX2fQ56y T9aA== 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=AFyySltaa8V9tju+y0URnlEXavXIT/655dGGhNpum2I=; b=NeffvxUfKRB2sD/Ci0+VcQnanB2DDKw6ZkEE9hUILJny6mN0HnBDxsP/F02vpU6MDO hpgfpKumW+lqBZQK4gJbJNViRznuGbpnmSiq7TnEKmzoC33fqRHRPD0t4JtiAmcIsrf2 LsUcH8OCfBAb+4AJJPEEbc83TS9KN2H1LDIlzKohyicakRCiqjHndAca8qtAhZc6BE/W vSCmlgkdQHjQvhQAJPXIjDHfBzt2Qw+D59LjzINzv/992seCXzlVUKaIEdCAyt1OlR+a nSJjRe5uE1zoG/rRxpsdt9vcqeY5dChsQT7oct8IEGE+Wafsgpa/QtIw3r0nujA8K2Ty QJ7g== X-Gm-Message-State: AOAM533JDmqXZiBcblADihVo67XRX+Kf/2cvNySKSZyX/bwnvXzLLmqr FQ2j2IErod2yng64/9OwXHqTbNZqxfZJCx2VZJVe6z1q X-Google-Smtp-Source: ABdhPJyMEtMS6JwrOOzvrwmpXlFYyKoQ/dUqCGBnVzXIOlZCBg2Q/AuckvOnmO1Llq68Q87o7Y0Tb4FGud4fzxdgc94= X-Received: by 2002:a05:6830:1682:: with SMTP id k2mr16042112otr.154.1613988317192; Mon, 22 Feb 2021 02:05:17 -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:200554 Archived-At: On Mon, Feb 22, 2021 at 9:38 AM Andrea Corallo wrote: > Pip Cet writes: > > On Sun, Feb 21, 2021 at 9:03 PM Andrea Corallo wrote: > >> 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. > > > > Thanks for your explanation! > > > >> 'and' is meant to imply intersection, so yeah... as you guess there's > >> some problem in the LIMPLE we generate :) > > > > Thanks, I was on the wrong track there. > > > >> 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 41665638 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. > > > > Okay, I think I understand the problem (we don't do classical SSA and > > throw away the slot numbers), and your solution would avoid it, but it > > seems needlessly complicated to me. > > Correct, ATM the assumption is that we keep LIMPLE always as > "conventional SSA form". This for a number of reasons but mainly it > greatly helps in maintaining the compiler simple. "Conventional" meaning "not quite SSA"? I'm just trying to understand, the decision seems correct to me, since we already ran stack slot allocation in the byte compiler and we want to reuse those assignments. > > Why create a new slot that isn't used anywhere? The value of the stack > > slot is clobbered by the (set ...), so we cannot emit any assumptions > > about that stack slot based on previous values it held. > > 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. > 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. IIRC we already do something similar for the -1 index so this > concept has just to be generalized a bit. 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. > > 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? > BTW applying your patch > is breaking quite some of the comp-tests-ret-type-spec-* tests :) Where do you keep those? I see the same number of test failures with and without the patch, running "make check". The failures seem unrelated, too...