GNU bug report logs - #46670
28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> gmail.com>
To: Andrea Corallo <akrl <at> sdf.org>
Cc: 46670 <at> debbugs.gnu.org, Mauricio Collares <mauricio <at> collares.org>
Subject: bug#46670: 28.0.50; [feature/native-comp] possible miscompilation affecting lsp-mode
Date: Tue, 23 Feb 2021 09:07:26 +0000

On Mon, Feb 22, 2021 at 11:16 AM Andrea Corallo <akrl <at> sdf.org> wrote:
> Pip Cet <pipcet <at> gmail.com> 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.

> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.