From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#43725: 28.0.50; Include feature/native-comp into master Date: Sat, 13 Feb 2021 22:06:51 +0200 Message-ID: <831rdjd95w.fsf@gnu.org> References: <87wny8xwcc.fsf@gnus.org> <83im9sqk1b.fsf@gnu.org> <83v9drp8va.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="5402"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, monnier@iro.umontreal.ca, 43725@debbugs.gnu.org To: akrl@sdf.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Feb 13 21:08:26 2021 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 1lB1DN-0001Hc-GV for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 13 Feb 2021 21:08:25 +0100 Original-Received: from localhost ([::1]:36760 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lB1DM-00051G-Jz for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 13 Feb 2021 15:08:24 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:35214) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lB1D0-000511-Mu for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2021 15:08:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:51791) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lB1D0-0000Kp-Fv for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2021 15:08:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lB1D0-0003zg-AR for bug-gnu-emacs@gnu.org; Sat, 13 Feb 2021 15:08:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 13 Feb 2021 20:08:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 43725 X-GNU-PR-Package: emacs Original-Received: via spool by 43725-submit@debbugs.gnu.org id=B43725.161324682615286 (code B ref 43725); Sat, 13 Feb 2021 20:08:02 +0000 Original-Received: (at 43725) by debbugs.gnu.org; 13 Feb 2021 20:07:06 +0000 Original-Received: from localhost ([127.0.0.1]:35104 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lB1C6-0003yU-9F for submit@debbugs.gnu.org; Sat, 13 Feb 2021 15:07:06 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:45150) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lB1C4-0003xu-SU for 43725@debbugs.gnu.org; Sat, 13 Feb 2021 15:07:05 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:56676) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lB1Bz-0000Bw-IN; Sat, 13 Feb 2021 15:06:59 -0500 Original-Received: from 84.94.185.95.cable.012.net.il ([84.94.185.95]:3655 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lB1Bv-0007cM-VY; Sat, 13 Feb 2021 15:06:56 -0500 In-Reply-To: <83v9drp8va.fsf@gnu.org> (message from Eli Zaretskii on Fri, 27 Nov 2020 09:16:41 +0200) 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" Xref: news.gmane.io gmane.emacs.bugs:199957 Archived-At: Andrea, can you please explain why there are so many changes in Lisp files on the branch? For example, what are changes like this one about: +(declare-function subr-native-lambda-list "data.c") Or this: + ((and (featurep 'nativecomp) + (subrp def) + (listp (subr-native-lambda-list def))) + (subr-native-lambda-list def)) Or this: + ;; Never native compile to allow cc-defs.el:2345 hack. + (declare (speed -1)) This will not work on MS-Windows, you need to use path-separator to do it portably: + (when (featurep 'nativecomp) + (defvar comp-eln-load-path) + (let ((path-env (getenv "EMACSNATIVELOADPATH"))) + (when path-env + (dolist (path (split-string path-env ":")) <<<<<<<<<<<<< + (unless (string= "" path) + (push path comp-eln-load-path))))) + (push (concat user-emacs-directory "eln-cache/") comp-eln-load-path)) I'm not sure I understand this addition to epaths.nt: +/* Like PATH_LOADSEARCH, but contains the relative path from the + installation directory. +*/ +#define PATH_REL_LOADSEARCH "" Can you explain how PATH_REL_LOADSEARCH is supposed to work, and for what purpose it was introduced? A few minor comments about the C code: This is unsafe, because a fixnum can be larger than PTRDIFF_MAX: +static gcc_jit_lvalue * +emit_mvar_lval (Lisp_Object mvar) +{ + Lisp_Object mvar_slot = CALL1I (comp-mvar-slot, mvar); + + if (EQ (mvar_slot, Qscratch)) + { + if (!comp.scratch) + comp.scratch = gcc_jit_function_new_local (comp.func, + NULL, + comp.lisp_obj_type, + "scratch"); + return comp.scratch; + } + + return comp.frame[XFIXNUM (mvar_slot)]; <<<<<<<<<<<<<<<<<<<< +} Likewise, this is unsafe because a fixnum can be larger than INT_MAX: + if (!FIXNUMP (idx)) + xsignal1 (Qnative_ice, + build_string ("inconsistent data relocation container")); + reloc.idx = gcc_jit_context_new_rvalue_from_int (comp.ctxt, + comp.ptrdiff_type, + XFIXNUM (idx)); <<<<<<<< (There are several more calls with the same problem.) Several comparisons like this one: + if (val != (long) val) are IMO better written as if (val > LONG_MAX || val < LONG_MIN) Here, wouldn't it be better to have an assertion that there are no more than 6 elements in the list: + Lisp_Object arg[6]; + + Lisp_Object p = XCDR (insn); + ptrdiff_t i = 0; + FOR_EACH_TAIL (p) + { + if (i == sizeof (arg) / sizeof (Lisp_Object)) + break; + arg[i++] = XCAR (p); + } If there are more than 6, we will be writing beyond the end of the arg[] array. This is nonportable: + if (!noninteractive) + { + sigset_t blocked; + /* Gcc doesn't like being interrupted at all. */ + block_input (); + sigemptyset (&blocked); + sigaddset (&blocked, SIGALRM); + sigaddset (&blocked, SIGINT); +#ifdef USABLE_SIGIO + sigaddset (&blocked, SIGIO); +#endif + pthread_sigmask (SIG_BLOCK, &blocked, &saved_sigset); <<<<<<<<<<< + count = SPECPDL_INDEX (); + record_unwind_protect_void (restore_sigmask); + } We shouldn't use pthread_sigmask unconditionally, we should use it only on Posix platforms. Can you explain why the signals here should be blocked? What happens if they aren't, and a signal arrives while the compilation runs? I'm asking because on MS-Windows blocking signals with sigaddset/sigmask doesn't really work, so the question is what if anything should be done here on Windows. Here, 'i' could be ptrdiff_t, no need to use EMACS_INT: + EMACS_INT d_vec_len = XFIXNUM (Flength (comp_u->data_vec)); + for (EMACS_INT i = 0; i < d_vec_len; i++) + if (!EQ (data_relocs[i], AREF (comp_u->data_vec, i))) + return false; + + d_vec_len = XFIXNUM (Flength (comp_u->data_impure_vec)); + for (EMACS_INT i = 0; i < d_vec_len; i++) This should have a big FIXME, since it means the name of the DLL is in two different places, and if we need to support a differently-named DLL, we are in trouble: +#ifdef WINDOWSNT + /* We may need to load libgccjit when dumping before term/w32-win.el + defines `dynamic-library-alist`. This will fail if that variable + is empty, so add libgccjit-0.dll to it. */ + if (will_dump_p ()) + Vdynamic_library_alist = list1 (list2 (Qgccjit, + build_string ("libgccjit-0.dll"))); Is there a more elegant way of resolving this difficulty? This lacks the ENCODE_FILE part: + char *fname = SSDATA (concat2 (Vinvocation_directory, + XCAR (comp_u->file))); + FILE *file; + if ((file = fopen (fname, "r"))) And why are you using fopen instead of emacs_fopen?