From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#67005: 30.0.50; improve nadivce/comp/trampoline handling Date: Tue, 14 Nov 2023 19:06:38 -0500 Message-ID: References: <874jhv9921.fsf@sappc2.fritz.box> <875y24zrt1.fsf@sappc2.fritz.box> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2294"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: andrea corallo , 67005@debbugs.gnu.org To: Jens Schmidt Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Nov 15 01:10:50 2023 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 1r33UX-0000Qn-Hk for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 15 Nov 2023 01:10:49 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1r33UB-00015o-4i; Tue, 14 Nov 2023 19:10:27 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r33U4-000147-Jn for bug-gnu-emacs@gnu.org; Tue, 14 Nov 2023 19:10:21 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1r33U4-00019S-BB for bug-gnu-emacs@gnu.org; Tue, 14 Nov 2023 19:10:20 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1r33Uj-0004SM-N5 for bug-gnu-emacs@gnu.org; Tue, 14 Nov 2023 19:11:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 15 Nov 2023 00:11:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67005 X-GNU-PR-Package: emacs Original-Received: via spool by 67005-submit@debbugs.gnu.org id=B67005.170000700317040 (code B ref 67005); Wed, 15 Nov 2023 00:11:01 +0000 Original-Received: (at 67005) by debbugs.gnu.org; 15 Nov 2023 00:10:03 +0000 Original-Received: from localhost ([127.0.0.1]:34297 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r33Tj-0004QN-3t for submit@debbugs.gnu.org; Tue, 14 Nov 2023 19:10:02 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:15757) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1r33Tg-0004Q9-F1 for 67005@debbugs.gnu.org; Tue, 14 Nov 2023 19:09:58 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 928C6442676; Tue, 14 Nov 2023 19:09:08 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1700006946; bh=paItBKZudM5SN3sYWUaQGogKx/0JCUobuhpYiEFmBV4=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=cJaHkC3r8J1gjBO3sEoJ1k/Bw5Vl1WYOBc8wsZfV8Z6dSqrT7g0tlcSoAqtZPCC04 6usyuswBLoHlQxKzfCjIbaOLQvVJvgOKGExt4hNYlsGNLar5d4oGZKA8IdIqqpA88V S9do9nrYzEqUzDw2agDsuoCxAUrl32A4NuGE5GX14EGeAUvHTx/5RiWlJUHDntVpEC u1qPLCVyPKrK16w4Hdq4VXOmLhaa+Kp106qXY1Jvs2s16jZbJdHxMGxnD/t83tbEDd f6s7d7GcTyDzw0+p3/99UdefjInOZx7CtOy4FFQ8a98xyg9mt1JxwfVBeb2yNl+8RQ lC/50E13SO0vQ== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 9465C442F9B; Tue, 14 Nov 2023 19:09:06 -0500 (EST) Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 7F30A1201F1; Tue, 14 Nov 2023 19:09:06 -0500 (EST) In-Reply-To: <875y24zrt1.fsf@sappc2.fritz.box> (Jens Schmidt's message of "Tue, 14 Nov 2023 09:02:02 +0100") 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:274334 Archived-At: > 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