unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Pip Cet <pipcet@gmail.com>
To: Andrea Corallo <akrl@sdf.org>
Cc: 46670@debbugs.gnu.org, Mauricio Collares <mauricio@collares.org>
Subject: bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode
Date: Mon, 22 Feb 2021 10:04:41 +0000	[thread overview]
Message-ID: <CAOqdjBeWdNwxW8cxr1VutPtf7vmWat3CHNJwguaMXLxDcJZ_=w@mail.gmail.com> (raw)
In-Reply-To: <xjfft1ol9xo.fsf@sdf.org>

On Mon, Feb 22, 2021 at 9:38 AM Andrea Corallo <akrl@sdf.org> wrote:
> Pip Cet <pipcet@gmail.com> writes:
> > On Sun, Feb 21, 2021 at 9:03 PM Andrea Corallo <akrl@sdf.org> wrote:
> >> Pip Cet <pipcet@gmail.com> writes:
> >>
> >> > On Sun, Feb 21, 2021 at 11:51 AM Pip Cet <pipcet@gmail.com> 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...





  reply	other threads:[~2021-02-22 10:04 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-21  0:12 bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode Mauricio Collares
2021-02-21 11:51 ` Pip Cet
2021-02-21 11:56   ` Pip Cet
2021-02-21 21:03     ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-21 22:46       ` Pip Cet
2021-02-22  9:37         ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-22 10:04           ` Pip Cet [this message]
2021-02-22 10:25             ` Pip Cet
2021-02-22 11:23               ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-22 12:11                 ` Pip Cet
2021-02-22 13:12                   ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-23  7:59                     ` Pip Cet
2021-02-23  9:04                       ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-23 23:26                         ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-24  2:10                           ` Mauricio Collares
2021-02-24  8:22                             ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-23 19:09                     ` Pip Cet
2021-02-23 23:36                       ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-24  4:31                         ` Pip Cet
2021-02-24  9:04                           ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-24  9:28                             ` Pip Cet
2021-02-24  9:42                               ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-24  9:46                                 ` Pip Cet
2021-02-24 10:06                                   ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-25 12:41                                     ` Pip Cet
2021-02-25 14:58                                       ` Eli Zaretskii
2021-02-25 15:14                                         ` Pip Cet
2021-02-25 15:31                                           ` Eli Zaretskii
2021-02-25 16:56                                       ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-25 20:59                                         ` Pip Cet
2021-02-26 19:33                                           ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-26 20:30                                             ` Pip Cet
2021-02-26 20:44                                               ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-26 20:11                                           ` Eli Zaretskii
2021-02-26 20:32                                             ` Pip Cet
2021-02-27  5:06                                             ` Pip Cet
2021-02-27  7:49                                               ` Eli Zaretskii
2021-02-27  9:39                                                 ` Pip Cet
2021-02-27 10:24                                                   ` Eli Zaretskii
2021-02-27 12:39                                                     ` Pip Cet
2021-02-27 13:30                                                       ` Eli Zaretskii
2021-02-27 17:15                                                         ` Pip Cet
2021-02-27 18:40                                                           ` Eli Zaretskii
2021-02-28  8:14                                                             ` Pip Cet
2021-03-01  5:24                                                               ` Richard Stallman
2021-03-01  6:40                                                                 ` Pip Cet
2021-02-22 11:16             ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-23  9:07               ` Pip Cet
2021-02-23 22:55                 ` Andrea Corallo via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-02-24  7:00                   ` Pip Cet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOqdjBeWdNwxW8cxr1VutPtf7vmWat3CHNJwguaMXLxDcJZ_=w@mail.gmail.com' \
    --to=pipcet@gmail.com \
    --cc=46670@debbugs.gnu.org \
    --cc=akrl@sdf.org \
    --cc=mauricio@collares.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).