bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Monnier
Subject: bug#67005: 30.0.50; improve nadivce/comp/trampoline handling
Date: Wed, 08 Nov 2023 17:50:58 -0500
User-agent: Gnus/5.13 (Gnus v5.13)

> 1. Since Stefan has removed the advices on uniquify functions in
>    1a724cc2d2e7, that special handling of `rename-buffer' in
>    `advice--add-function' and `native-comp-never-optimize-functions'
>    seems to be obsolete.  Away with it:

+1

> 2. The comment in `advice--add-function' says that `macroexpand' causes
>    a circular dependency, and there *are* still advices done on
>    `macroexpand' (at least in `cl-symbol-macrolet').  But probably these
>    are not executed during the bootstrap?  Let's try.

+1

> 3. But then on the other hand, function Ffset also calls
>    `comp-subr-trampoline-install', and if we have condition
>
>      `(subr-primitive-p (gv-deref ref))'
>
>    fulfilled in `advice--add-function', then the following `(setf
>    (gv-deref ref) ...)' should ultimately call `fset'.  So probably we
>    can completely remove that `comp-subr-trampoline-install' call from
>    `advice--add-function', like this:

It looks good to me, tho I must admit I do not understand why we have
this `comp-subr-trampoline-install` in addition to the one in `Ffset`,
so maybe I'm missing something.

> 5. And actually I think that b) above probably is a bug: It means that
>    for advised subrs, the optimzation to indirect calls in function
>    `comp-call-optim-form-call' never takes place.
>
>    So I tried to recognize subrs in function `comp-call-optim-form-call'
>    even when they are advised:

I think this is not a correct optimization, as you mention:

>    But: Natively compiled code now does NOT execute the advice on
>    `rename-buffer' added in step 4, since its call gets optimized but no
>    trampoline has been created for it.

So it's a -1 from me on this patch.

> 6. So there is the question whether we should actively disallow advices
>    during bootstrap, now that we are free of them.  Like this:

Rather than `dump-mode`, I'd test `purify-flag`.
This is because `purify-flag` is not set while building `src/bootstrap-emacs`.

Part of the issue here is that during the build of `src/bootstrap-emacs`
we load a lot more ELisp code than in the "real" build so it's good to
restrict this constraint to the "real build".


        Stefan






reply via email to

[Prev in Thread] Current Thread [Next in Thread]