bug#67005: 30.0.50; improve nadivce/comp/trampoline handling

[Stefan Monnier]

> There are two different items in the native compilation area called
> "trampoline":
>
> - There are "funcall trampolines", which are instructions to call a
>   function from natively compiled code through a `funcall' indirection.
>   A call of a primitive through such a funcall trampoline is pretty
>   close to a function call of that primitive from interpreted Elisp and
>   always executes advices.

IIUC this is not a separate piece of code, tho.
It's just the use of the "normal" `funcall` instead of calling
a `subr` directly.

E.g. this is not something installed with `comp-subr-trampoline-install`.

> - And there are "native trampolines", which are natively compiled
>   wrappers for primitives.  If and only if a primitive has such a native
>   trampoline, advices on the primitive are executed even if the
>   primitive is called from natively compiled code *without* a funcall
>   trampoline.

And IIUC these are the thingies manipulated/generated/installed with
`comp-subr-trampoline-install`.

> ------------------------- LIMPEL -------------------------
> Pre comp-call-optim
>
> (set #(mvar 23578551540518 1 float) (callref funcall #(mvar 23578553446684 1 
> (member sqrt)) #(mvar 23578554498624 2 t)))
>                                      
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

IIUC this generates a piece of native call which will use `Ffuncall`
passing it the symbol `sqrt`, so it will always behave exactly like
the byte-compiler would.

> Post comp-call-optim
>
> (set #(mvar 23578551540518 1 float) (call sqrt #(mvar 23578554498624 2 t)))
>                                      ^^^^
> ------------------------- LIMPEL -------------------------

This instead generates a piece of code which calls the `Fsqrt` function
via a C-level function pointer kept in a `link_table`.

> So the indirect `callref funcall' got optimized to a direct `call', and:
> An advice on `sqrt' is executed even if the natively compiled `foo' is
> called:
>
> ------------------------- snip(B) -------------------------
> (defun sqrt-advice (&rest _)
>   (message "Advice called"))
> (advice-add #'sqrt :before #'sqrt-advice)
> (foo 0)
>
> => "Advice called" echoed in message area
> ------------------------- snip(B) -------------------------

Indeed, when an advice is installed `comp-subr-trampoline-install` is
called which replaces the pointer to `Fsqrt` to a pointer to
a "trampoline" which calls `Ffuncall` with `sqrt` as argument instead.

> Now re-evaluate the snip(A) again and see how `foo' is *not* optimized,
> this time because of the advice on `sqrt' and because of what I consider
> the bug in `comp-call-optim-form-call':
>
> ------------------------- LIMPEL -------------------------
> Pre comp-call-optim
>
> (set #(mvar 23487733705560 1 float) (callref funcall #(mvar 23487733185614 1 
> (member sqrt)) #(mvar 23487733838350 2 t)))
>                                      
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Post comp-call-optim
>
> (set #(mvar 23487733705560 1 float) (callref funcall #(mvar 23487733185614 1 
> (member sqrt)) #(mvar 23487733838350 2 t)))
>                                      
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This seems OK: if there's an advice when the code is compiled, there's
a good chance that the advice will also be present when the code is
executed, in which case it's more efficient to use a straight `funcall`
than a "direct" call that gets redirected to `funcall`.

BTW, this direct call and trampoline business becomes even more fun when
you consider things like:

    (defun my-fun1 ...)
    (defalias 'my-fun2 (symbol-function 'my-fun1))

in which case both `my-fun1` and `my-fun2` will contain the same subr,
so you'd ideally want to generate direct calls both for calls to
`my-fun1` and for calls to `my-fun2` but they can't use the same slot in
the `link_table` because they have to obey separately to redefinitions
`my-fun1` or `my-fun2`.

> So for me the bigger picture or problem is that it is not always enough
> to rely on a simple `(symbol-function 'foo)' when you want to process
> function `foo'.  It is enough if you just want to call it, but not if
> you want to do fancy things with it, like compiling it.

Hmm... in the example above, we don't want to compile it, we just want
to generate an efficient call to it.

> In these fancy cases you might really want to dig through all possibly
> existing advices on `foo' and process the _original_ function instead.

While it could occasionally be beneficial, I think in more cases it'll
be detrimental.  And in any case this is a very rare occurrence so the
choice probably doesn't actually matter at all in practice.

> ------------------------- snip -------------------------
> (defun bar () nil)
> (advice-add 'bar :before 'ignore)
> (native-compile 'bar)
>
> =>
>
> comp--native-compile: Native compiler error: bar, "can't native compile an 
> already byte-compiled function"
> ------------------------- snip -------------------------

Here the question is: what do you mean by (native-compile 'bar)?
Do you mean to compile the advice wrapper (a byte-compiled function)
or some of its content (and if so which one(s))?

We could tweak `native-compile` and `byte-compile` so they look through
pieces of advice to try and find the underlying not-yet-compiled
functions, compile them and re-install them where they were found, but
obviously, noone asked for that until now and hence noone bothered to do
that work, most likely because things like (native-compile 'bar) and
(byte-compile 'bar) are operations used extremely rarely.

> Another suspectible spot is the following from function `Ffset':
>
>   if (!NILP (Vnative_comp_enable_subr_trampolines)
>       && SUBRP (function)
>       && !SUBR_NATIVE_COMPILEDP (function))
>     CALLN (Ffuncall, Qcomp_subr_trampoline_install, symbol);

Actually, I don't understand this code now that I re-(re-)*read it.
Why do we negate the SUBR_NATIVE_COMPILEDP (function)?


        Stefan