unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: scratch/comp-static-data b5fc7dabb5 2/2: src/alloc.h: Add new header.
       [not found] ` <20221119164403.6C791C00B0F@vcs2.savannah.gnu.org>
@ 2022-11-20  0:41   ` Po Lu
  0 siblings, 0 replies; 2+ messages in thread
From: Po Lu @ 2022-11-20  0:41 UTC (permalink / raw)
  To: emacs-devel; +Cc: Vibhav Pant

Vibhav Pant <vibhavp@gmail.com> writes:

> branch: scratch/comp-static-data
> commit b5fc7dabb5c153336bd9268c80d9316faa34753a
> Author: Vibhav Pant <vibhavp@gmail.com>
> Commit: Vibhav Pant <vibhavp@gmail.com>
>
>     src/alloc.h: Add new header.
>     
>     Add alloc.h, a header containing allocation-related constants that are
>     used by comp.c while emitting static data.
>     * src/alloc.h: New file.
>     * src/alloc.c: Set alloc.h variables only when native compilation is
>       enabled.

You wrote the commit message wrong again.
The last line should _not_ be indented to line up with "src/alloc.c".

Also, why not put this stuff in lisp.h? That's how it's done in the tree
where I work on the garbage collector, and it seems to be working fine.
Less headers are better than more.

> ---
>  src/alloc.c |  6 ++++++
>  src/alloc.h | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/src/alloc.c b/src/alloc.c
> index 69c5dba18c..6e46d470cf 100644
> --- a/src/alloc.c
> +++ b/src/alloc.c
> @@ -1054,7 +1054,9 @@ lisp_free (void *block)
>  #define BLOCK_ALIGN (1 << 10)
>  verify (POWER_OF_2 (BLOCK_ALIGN));
>  
> +#ifdef HAVE_NATIVE_COMP
>  const size_t block_align = BLOCK_ALIGN;
> +#endif
>  
>  /* Use aligned_alloc if it or a simple substitute is available.
>     Aligned allocation is incompatible with unexmacosx.c, so don't use
> @@ -2547,9 +2549,11 @@ pin_string (Lisp_Object string)
>       - (sizeof (struct Lisp_Float) - sizeof (bits_word))) * CHAR_BIT) \
>     / (sizeof (struct Lisp_Float) * CHAR_BIT + 1))
>  
> +#ifdef HAVE_NATIVE_COMP
>  const size_t float_block_floats_length = FLOAT_BLOCK_SIZE;
>  const size_t float_block_gcmarkbits_length
>    = 1 + FLOAT_BLOCK_SIZE / BITS_PER_BITS_WORD;
> +#endif
>  
>  #define GETMARKBIT(block,n)				\
>    (((block)->gcmarkbits[(n) / BITS_PER_BITS_WORD]	\
> @@ -2655,9 +2659,11 @@ make_float (double float_value)
>       - (sizeof (struct Lisp_Cons) - sizeof (bits_word))) * CHAR_BIT)	\
>     / (sizeof (struct Lisp_Cons) * CHAR_BIT + 1))
>  
> +#ifdef HAVE_NATIVE_COMP
>  const size_t cons_block_conses_length = CONS_BLOCK_SIZE;
>  const size_t cons_block_gcmarkbits_length
>    = 1 + CONS_BLOCK_SIZE / BITS_PER_BITS_WORD;
> +#endif
>  
>  #define CONS_BLOCK(fptr) \
>    (eassert (!pdumper_object_p (fptr)),                                  \
> diff --git a/src/alloc.h b/src/alloc.h
> new file mode 100644
> index 0000000000..026d62c6a9
> --- /dev/null
> +++ b/src/alloc.h
> @@ -0,0 +1,35 @@
> +/* Allocation-related definitions, used by comp.c
> +
> + Copyright (C) 2019-2022 Free Software Foundation, Inc.
> +
> +This file is part of GNU Emacs.
> +
> +GNU Emacs is free software: you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation, either version 3 of the License, or
> +(at your option) any later version.
> +
> +GNU Emacs is distributed in the hope that it will be useful,
> +but WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef ALLOC_H
> +#define ALLOC_H
> +
> +#ifdef HAVE_NATIVE_COMP
> +
> +extern const size_t block_align;
> +
> +extern const size_t float_block_floats_length;
> +extern const size_t float_block_gcmarkbits_length;
> +
> +extern const size_t cons_block_conses_length;
> +extern const size_t cons_block_gcmarkbits_length;
> +
> +#endif /* #ifndef HAVE_NATIVE_COMP */
> +
> +#endif /* #ifndef ALLOC_H */



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: scratch/comp-static-data 87a249440f 1/2: src/comp.c: Use `long' rvalues for storing values with GC bits.
       [not found] ` <20221119164403.530B7C009EC@vcs2.savannah.gnu.org>
@ 2022-11-20  0:45   ` Po Lu
  0 siblings, 0 replies; 2+ messages in thread
From: Po Lu @ 2022-11-20  0:45 UTC (permalink / raw)
  To: emacs-devel; +Cc: Vibhav Pant

Vibhav Pant <vibhavp@gmail.com> writes:

>     * src/comp.c (emit_lisp_string_constructor_rval,
>       float_block_emit_constructor, emit_comp_lisp_obj): Use
>       `gcc_jit_context_new_rvalue_from_long' to ensure that GC bits are
>       correctly encoded in any ptrdiff_t rvalue.

Here, you wrapped the files _inside_ the parentheses in the first line
of the commit message.  Not OK!  It should be:

* src/comp.c (emit_lisp_string_constructor_rval)
(float_block_emit_constructor, emit_comp_lisp_obj): Use
`gcc_jit_context_new_rvalue_from_long' to ensure that GC
bits are correctly encoded in any ptrdiff_t value.

In addition, did you check this on a build with wide ints on a 32 bit
machine?  Maybe even one with 68000 byte order?

>    gcc_jit_rvalue *size_bytes
>      = STRING_MULTIBYTE (str)
> -	? gcc_jit_context_new_rvalue_from_int (comp.ctxt,
> +	? gcc_jit_context_new_rvalue_from_long (comp.ctxt,
>  					       comp.ptrdiff_type,
>  					       SBYTES (str))
>  	// Mark unibyte strings as immovable, so that pin_string does
>  	// not attempt to modify them.
> -	: gcc_jit_context_new_rvalue_from_int (comp.ctxt,
> +	: gcc_jit_context_new_rvalue_from_long (comp.ctxt,
>  					       comp.ptrdiff_type, -3);

Would you please place parentheses around the whole ternary, and indent
accordingly?

Thanks.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-11-20  0:45 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <166887624274.20851.208849360159299364@vcs2.savannah.gnu.org>
     [not found] ` <20221119164403.6C791C00B0F@vcs2.savannah.gnu.org>
2022-11-20  0:41   ` scratch/comp-static-data b5fc7dabb5 2/2: src/alloc.h: Add new header Po Lu
     [not found] ` <20221119164403.530B7C009EC@vcs2.savannah.gnu.org>
2022-11-20  0:45   ` scratch/comp-static-data 87a249440f 1/2: src/comp.c: Use `long' rvalues for storing values with GC bits Po Lu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).