From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?utf-8?Q?Mattias_Engdeg=C3=A5rd?= Newsgroups: gmane.emacs.devel Subject: Re: master 3ed79cd: Separate bytecode stack Date: Tue, 15 Mar 2022 15:20:29 +0100 Message-ID: <210D2D02-9086-4DB5-8015-120E07489974@acm.org> References: <83lexdlpjy.fsf@gnu.org> <447E1D53-FA02-4776-9730-507BCA1993FA@acm.org> <83h781lma9.fsf@gnu.org> <1EB42282-A67C-4C20-8E5D-BA8DA9A21E9C@acm.org> <83a6dsjw12.fsf@gnu.org> Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.120.0.1.13\)) Content-Type: multipart/mixed; boundary="Apple-Mail=_81F6B086-8846-484E-988E-48A2FD14F8E5" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25264"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Emacs-Devel To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Mar 15 15:22:10 2022 Return-path: Envelope-to: ged-emacs-devel@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 1nU83u-0006OK-DR for ged-emacs-devel@m.gmane-mx.org; Tue, 15 Mar 2022 15:22:10 +0100 Original-Received: from localhost ([::1]:44706 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nU83s-00046V-PH for ged-emacs-devel@m.gmane-mx.org; Tue, 15 Mar 2022 10:22:08 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:52918) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nU82T-0001uP-AK for emacs-devel@gnu.org; Tue, 15 Mar 2022 10:20:41 -0400 Original-Received: from mail18c50.megamailservers.eu ([91.136.10.28]:56282) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nU82Q-0006eU-7X; Tue, 15 Mar 2022 10:20:40 -0400 X-Authenticated-User: mattiase@bredband.net DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=megamailservers.eu; s=maildub; t=1647354032; bh=fA1kv3rLg7ePO+X4mqTY7vGn3q7l9LeMwU4gLoATaw8=; h=From:Subject:Date:In-Reply-To:Cc:To:References:From; b=CyCp7kkNyLfWyfRgCb/csXaqA2x+2ZW9NP4k1lFJNPrYREM06cVLUwKyaxERmPEvP rHok71HR+l0L9aE2hgaCZZWJuC09fb5NuWnxKYNUhA/HffDNn4tNtYwE9TfZsA6ElG HfZs78bBgth1EH35LyPlGprtewQmxsxJ9ytzzvLc= Feedback-ID: mattiase@acm.or Original-Received: from smtpclient.apple (c188-150-171-71.bredband.tele2.se [188.150.171.71]) (authenticated bits=0) by mail18c50.megamailservers.eu (8.14.9/8.13.1) with ESMTP id 22FEKUrO017289; Tue, 15 Mar 2022 14:20:32 +0000 In-Reply-To: <83a6dsjw12.fsf@gnu.org> X-Mailer: Apple Mail (2.3654.120.0.1.13) X-CTCH-RefID: str=0001.0A742F23.6230A0B0.005F, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-CTCH-VOD: Unknown X-CTCH-Spam: Unknown X-CTCH-Score: 0.000 X-CTCH-Flags: 0 X-CTCH-ScoreCust: 0.000 X-Origin-Country: SE Received-SPF: softfail client-ip=91.136.10.28; envelope-from=mattiase@acm.org; helo=mail18c50.megamailservers.eu X-Spam_score_int: -11 X-Spam_score: -1.2 X-Spam_bar: - X-Spam_report: (-1.2 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, SPF_HELO_NONE=0.001, SPF_SOFTFAIL=0.665, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:287185 Archived-At: --Apple-Mail=_81F6B086-8846-484E-988E-48A2FD14F8E5 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii 14 mars 2022 kl. 18.15 skrev Eli Zaretskii : > I'm talking about the 'fp' array, where you store values at least some > of which seem to be pointers to Lisp_Object. But the storing function > treats them as integers: I did try to explain this but evidently without skill. Let me try again: What you call "the 'fp' array" is the fixed-size part of a stack frame, = which needs to store a few values of types `Lisp_Object`, `Lisp_Object = *` and `const unsigned char *` in slots of type `Lisp_Object`. This is = nothing unusual: we store pointers in Lisp_Object values everywhere in = Emacs and use XIL (etc) to do so. Don't get yourself hung up on the fact = that we take the route via integers; that's what we do for Lisp strings, = vectors etc as well. > Why do you need to do that? Why not store the pointer itself in the > first place, and make 'fp' be array of pointers to pointer to > Lisp_Object? Because we use a part of the overall stack which is an array of = Lisp_Object, and in C we can't cast between an array of unions and a = union of arrays -- or rather we could try, but if something as benighn = as storing pointers in a Lisp_Object makes you squeamish then this stunt = would be much worse (as well as technically dubious for all sorts of = reasons). > My second question was: if you do need to store sometimes a pointer > and sometimes a Lisp_Object that is actually an integer, why not use a > union that will allow you to do both cleanly and safely, in effect > doing all the type-casting and stuff for you? There appears to be a misunderstanding here. The data stack of each = stack frame must be an array of Lisp_Object; it cannot be anything else. = That's why the overall stack has that type; the frame metadata just uses = a few slots between each local data stack. Since the data stacks vary in = size, the location of the frame metadata in the overall stack isn't = statically known. I mentioned another possibility that is cleaner in some respects; I'm = attaching a tentative patch. It uses a bona fide struct for the metadata = and uses hand-placement of the data stacks. The generated code should be = identical but might actually be better with the patch for various = reasons. >> The latter alternative would become a little more palatable if we = could use flexible array struct members on all platforms. Given that we = assume C99, can we do that now? >=20 > What do you mean by "flexible array struct members"? Please show a > code snippet. I'm sure you know it well, it's the ability to put an incomplete array = declaration as the last member of a struct, as in: struct S { int n; float a[]; }; which roughly means the same thing as declaring that array to be of size = zero. It's not required in our case but would make the next_stack function = (see patch) go away. --Apple-Mail=_81F6B086-8846-484E-988E-48A2FD14F8E5 Content-Disposition: attachment; filename=stack-metadata-struct.diff Content-Type: application/octet-stream; x-unix-mode=0644; name="stack-metadata-struct.diff" Content-Transfer-Encoding: 7bit diff --git a/src/bytecode.c b/src/bytecode.c index 8704e6069d..139d37ef43 100644 --- a/src/bytecode.c +++ b/src/bytecode.c @@ -335,20 +335,7 @@ bcall0 (Lisp_Object f) Ffuncall (1, &f); } -/* Layout of the stack frame header. */ -enum stack_frame_index { - SFI_SAVED_FP, /* previous frame pointer */ - - /* In a frame called directly from C, the following two members are NULL. */ - SFI_SAVED_TOP, /* previous stack pointer */ - SFI_SAVED_PC, /* previous program counter */ - - SFI_FUN, /* current function object */ - - SF_SIZE /* number of words in the header */ -}; - -/* The bytecode stack size in Lisp words. +/* The bytecode stack size in bytes. This is a fairly generous amount, but: - if users need more, we could allocate more, or just reserve the address space and allocate on demand @@ -357,7 +344,7 @@ bcall0 (Lisp_Object f) - for maximum flexibility but a small runtime penalty, we could allocate the stack in smaller chunks as needed */ -#define BC_STACK_SIZE (512 * 1024) +#define BC_STACK_SIZE (512 * 1024 * sizeof (Lisp_Object)) /* Bytecode interpreter stack: @@ -385,51 +372,32 @@ #define BC_STACK_SIZE (512 * 1024) : : */ -INLINE void * -sf_get_ptr (Lisp_Object *fp, enum stack_frame_index index) -{ - return XLP (fp[index]); -} - -INLINE void -sf_set_ptr (Lisp_Object *fp, enum stack_frame_index index, void *value) -{ - fp[index] = XIL ((uintptr_t)value); -} - -INLINE Lisp_Object * -sf_get_lisp_ptr (Lisp_Object *fp, enum stack_frame_index index) -{ - return sf_get_ptr (fp, index); -} +/* bytecode stack frame header (footer, actually) */ +struct bc_frame { + struct bc_frame *saved_fp; /* previous frame pointer, + NULL if bottommost frame */ -INLINE void -sf_set_lisp_ptr (Lisp_Object *fp, enum stack_frame_index index, - Lisp_Object *value) -{ - sf_set_ptr (fp, index, value); -} + /* In a frame called directly from C, the following two members are NULL. */ + Lisp_Object *saved_top; /* previous stack pointer */ + const unsigned char *saved_pc; /* previous program counter */ -INLINE const unsigned char * -sf_get_saved_pc (Lisp_Object *fp) -{ - return sf_get_ptr (fp, SFI_SAVED_PC); -} + Lisp_Object fun; /* current function object */ +}; -INLINE void -sf_set_saved_pc (Lisp_Object *fp, const unsigned char *value) +INLINE Lisp_Object * +next_stack (struct bc_frame *fp) { - sf_set_ptr (fp, SFI_SAVED_PC, (unsigned char *)value); + return (Lisp_Object *)(fp + 1); } void init_bc_thread (struct bc_thread_state *bc) { - bc->stack = xmalloc (BC_STACK_SIZE * sizeof *bc->stack); + bc->stack = xmalloc (BC_STACK_SIZE); bc->stack_end = bc->stack + BC_STACK_SIZE; /* Put a dummy header at the bottom to indicate the first free location. */ - bc->fp = bc->stack; - memset (bc->fp, 0, SF_SIZE * sizeof *bc->stack); + bc->fp = (struct bc_frame *)bc->stack; + memset (bc->fp, 0, sizeof *bc->fp); } void @@ -441,16 +409,16 @@ free_bc_thread (struct bc_thread_state *bc) void mark_bytecode (struct bc_thread_state *bc) { - Lisp_Object *fp = bc->fp; + struct bc_frame *fp = bc->fp; Lisp_Object *top = NULL; /* stack pointer of topmost frame not known */ for (;;) { - Lisp_Object *next_fp = sf_get_lisp_ptr (fp, SFI_SAVED_FP); + struct bc_frame *next_fp = fp->saved_fp; /* Only the dummy frame at the bottom has saved_fp = NULL. */ if (!next_fp) break; - mark_object (fp[SFI_FUN]); - Lisp_Object *frame_base = next_fp + SF_SIZE; + mark_object (fp->fun); + Lisp_Object *frame_base = next_stack (next_fp); if (top) { /* The stack pointer of a frame is known: mark the part of the stack @@ -464,7 +432,7 @@ mark_bytecode (struct bc_thread_state *bc) /* The stack pointer is unknown -- mark everything conservatively. */ mark_memory (frame_base, fp); } - top = sf_get_lisp_ptr (fp, SFI_SAVED_TOP); + top = fp->saved_top; fp = next_fp; } } @@ -477,10 +445,10 @@ DEFUN ("internal-stack-stats", Finternal_stack_stats, Sinternal_stack_stats, struct bc_thread_state *bc = ¤t_thread->bc; int nframes = 0; int nruns = 0; - for (Lisp_Object *fp = bc->fp; fp; fp = sf_get_lisp_ptr (fp, SFI_SAVED_FP)) + for (struct bc_frame *fp = bc->fp; fp; fp = fp->saved_fp) { nframes++; - if (sf_get_lisp_ptr (fp, SFI_SAVED_TOP) == NULL) + if (fp->saved_top == NULL) nruns++; } fprintf (stderr, "%d stack frames, %d runs\n", nframes, nruns); @@ -491,8 +459,8 @@ DEFUN ("internal-stack-stats", Finternal_stack_stats, Sinternal_stack_stats, INLINE bool valid_sp (struct bc_thread_state *bc, Lisp_Object *sp) { - Lisp_Object *fp = bc->fp; - return sp < fp && sp + 1 >= sf_get_lisp_ptr (fp, SFI_SAVED_FP) + SF_SIZE; + struct bc_frame *fp = bc->fp; + return sp < (Lisp_Object *)fp && sp + 1 >= next_stack (fp->saved_fp); } /* Execute the byte-code in FUN. ARGS_TEMPLATE is the function arity @@ -532,20 +500,20 @@ exec_byte_code (Lisp_Object fun, ptrdiff_t args_template, Lisp_Object *vectorp = XVECTOR (vector)->contents; EMACS_INT max_stack = XFIXNAT (maxdepth); - Lisp_Object *frame_base = bc->fp + SF_SIZE; - Lisp_Object *fp = frame_base + max_stack; + Lisp_Object *frame_base = next_stack (bc->fp); + struct bc_frame *fp = (struct bc_frame *)(frame_base + max_stack); - if (fp + SF_SIZE > bc->stack_end) + if ((char *)next_stack (fp) > bc->stack_end) error ("Bytecode stack overflow"); /* Save the function object so that the bytecode and vector are held from removal by the GC. */ - fp[SFI_FUN] = fun; + fp->fun = fun; /* Save previous stack pointer and pc in the new frame. If we came directly from outside, these will be NULL. */ - sf_set_lisp_ptr (fp, SFI_SAVED_TOP, top); - sf_set_saved_pc (fp, pc); - sf_set_lisp_ptr (fp, SFI_SAVED_FP, bc->fp); + fp->saved_top = top; + fp->saved_pc = pc; + fp->saved_fp = bc->fp; bc->fp = fp; top = frame_base - 1; @@ -914,7 +882,7 @@ #define DEFINE(name, value) [name] = &&insn_ ## name, CASE (Breturn): { - Lisp_Object *saved_top = sf_get_lisp_ptr (bc->fp, SFI_SAVED_TOP); + Lisp_Object *saved_top = bc->fp->saved_top; if (saved_top) { Lisp_Object val = TOP; @@ -925,11 +893,11 @@ #define DEFINE(name, value) [name] = &&insn_ ## name, specpdl_ptr--; top = saved_top; - pc = sf_get_saved_pc (bc->fp); - Lisp_Object *fp = sf_get_lisp_ptr (bc->fp, SFI_SAVED_FP); + pc = bc->fp->saved_pc; + struct bc_frame *fp = bc->fp->saved_fp; bc->fp = fp; - Lisp_Object fun = fp[SFI_FUN]; + Lisp_Object fun = fp->fun; Lisp_Object bytestr = AREF (fun, COMPILED_BYTECODE); Lisp_Object vector = AREF (fun, COMPILED_CONSTANTS); bytestr_data = SDATA (bytestr); @@ -1004,9 +972,9 @@ #define DEFINE(name, value) [name] = &&insn_ ## name, handlerlist = c->next; top = c->bytecode_top; op = c->bytecode_dest; - Lisp_Object *fp = bc->fp; + struct bc_frame *fp = bc->fp; - Lisp_Object fun = fp[SFI_FUN]; + Lisp_Object fun = fp->fun; Lisp_Object bytestr = AREF (fun, COMPILED_BYTECODE); Lisp_Object vector = AREF (fun, COMPILED_CONSTANTS); bytestr_data = SDATA (bytestr); @@ -1756,7 +1724,7 @@ #define DEFINE(name, value) [name] = &&insn_ ## name, exit: - bc->fp = sf_get_lisp_ptr (bc->fp, SFI_SAVED_FP); + bc->fp = bc->fp->saved_fp; Lisp_Object result = TOP; return result; diff --git a/src/lisp.h b/src/lisp.h index 8053bbc977..d6cd72cd01 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3546,7 +3546,7 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs) sys_jmp_buf jmp; EMACS_INT f_lisp_eval_depth; specpdl_ref pdlcount; - Lisp_Object *act_rec; + struct bc_frame *act_rec; int poll_suppress_count; int interrupt_input_blocked; }; @@ -4861,14 +4861,14 @@ #define DAEMON_RUNNING (w32_daemon_event != INVALID_HANDLE_VALUE) extern void free_bc_thread (struct bc_thread_state *bc); extern void mark_bytecode (struct bc_thread_state *bc); -INLINE Lisp_Object * +INLINE struct bc_frame * get_act_rec (struct thread_state *th) { return th->bc.fp; } INLINE void -set_act_rec (struct thread_state *th, Lisp_Object *act_rec) +set_act_rec (struct thread_state *th, struct bc_frame *act_rec) { th->bc.fp = act_rec; } diff --git a/src/thread.h b/src/thread.h index a29af702d1..ddba1a2d99 100644 --- a/src/thread.h +++ b/src/thread.h @@ -35,9 +35,11 @@ #define THREAD_H /* Byte-code interpreter thread state. */ struct bc_thread_state { - Lisp_Object *fp; /* current frame pointer (see bytecode.c) */ - Lisp_Object *stack; - Lisp_Object *stack_end; + struct bc_frame *fp; /* current frame pointer */ + + /* start and end of allocated bytecode stack */ + char *stack; + char *stack_end; }; struct thread_state --Apple-Mail=_81F6B086-8846-484E-988E-48A2FD14F8E5--