Package: emacs;
Reported by: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Fri, 1 Jun 2012 23:40:02 UTC
Severity: minor
Tags: patch
Done: Paul Eggert <eggert <at> cs.ucla.edu>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: help-debbugs <at> gnu.org (GNU bug Tracking System) To: Paul Eggert <eggert <at> cs.ucla.edu> Subject: bug#11604: closed (Re: bug#11604: revised patch for Bug #11604) Date: Sun, 29 Jul 2012 07:37:01 +0000
[Message part 1 (text/plain, inline)]
Your bug report #11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup. which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 11604 <at> debbugs.gnu.org. -- 11604: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11604 GNU Bug Tracking System Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Paul Eggert <eggert <at> cs.ucla.edu> To: 11604-done <at> debbugs.gnu.org Subject: Re: bug#11604: revised patch for Bug #11604 Date: Sun, 29 Jul 2012 00:28:55 -0700In trunk bzr 108598 Andreas Schwab fixed the problem in a nicer way than in my proposed patch, so I'm marking this as done. Thanks, Andreas.
[Message part 3 (message/rfc822, inline)]
From: Paul Eggert <eggert <at> cs.ucla.edu> To: bug-gnu-emacs <at> gnu.org Subject: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup. Date: Fri, 01 Jun 2012 16:37:15 -0700Tags: patch Here's a patch I'd like to install into the trunk soon, after testing it on a few more platforms. It follows up on a recent discussion in emacs-devel. === modified file 'src/ChangeLog' --- src/ChangeLog 2012-06-01 20:49:03 +0000 +++ src/ChangeLog 2012-06-01 23:34:07 +0000 @@ -1,5 +1,25 @@ 2012-06-01 Paul Eggert <eggert <at> cs.ucla.edu> + USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup. + * alloc.c (make_number) [!defined make_number]: + Remove, as lisp.h always defines this now. + (mark_maybe_pointer): Simplify since USE_LSB_TAG is always defined now. + * data.c (Fmake_variable_buffer_local, Fmake_local_variable): + * ftfont.c (ftfont_driver): Use LISP_INITIALLY_ZERO. + * lisp.h (USE_LSB_TAG): Allow the builder to compile with + -DUSE_LSB_TAG=0, to override the automatically-selected default. + USE_LSB_TAG now is always defined to be either 0 or 1. + All uses changed. + (union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the + code works fine either way, and efficiency is not a concern here. + (LISP_MAKE_RVALUE, make_number) [USE_LISP_UNION_TYPE]: + Use an inline function on all platforms, since this is simpler and + 'static inline' (via gnulib) is portable now. + (LISP_INITIALLY_ZERO): New macro. + (XSET) [USE_LISP_UNION_TYPE]: Don't overparenthesize. + * w32heap.c (allocate_heap, init_heap): + Pay no attention to USE_LISP_UNION_TYPE, as it's irrelevant here. + * xfns.c (x_set_tool_bar_lines) [USE_GTK]: Adjust to bitfield change. 2012-06-01 Dmitry Antipov <dmantipov <at> yandex.ru> === modified file 'src/alloc.c' --- src/alloc.c 2012-05-30 07:59:44 +0000 +++ src/alloc.c 2012-06-01 23:34:07 +0000 @@ -525,7 +525,7 @@ char c; \ }, \ c) -#ifdef USE_LSB_TAG +#if USE_LSB_TAG /* A common multiple of the positive integers A and B. Ideally this would be the least common multiple, but there's no way to do that as a constant expression in C, so do the best that we can easily do. */ @@ -890,8 +890,8 @@ number of bytes to allocate, TYPE describes the intended use of the allocated memory block (for strings, for conses, ...). */ -#ifndef USE_LSB_TAG -static void *lisp_malloc_loser; +#if ! USE_LSB_TAG +void *lisp_malloc_loser EXTERNALLY_VISIBLE; #endif static void * @@ -907,7 +907,7 @@ val = (void *) malloc (nbytes); -#ifndef USE_LSB_TAG +#if ! USE_LSB_TAG /* If the memory just allocated cannot be addressed thru a Lisp object's pointer, and it needs to be, that's equivalent to running out of memory. */ @@ -1088,7 +1088,7 @@ mallopt (M_MMAP_MAX, MMAP_MAX_AREAS); #endif -#ifndef USE_LSB_TAG +#if ! USE_LSB_TAG /* If the memory just allocated cannot be addressed thru a Lisp object's pointer, and it needs to be, that's equivalent to running out of memory. */ @@ -1581,20 +1581,6 @@ if (! NULL_INTERVAL_P (i)) \ (i) = balance_intervals (i); \ } while (0) - - -/* Number support. If USE_LISP_UNION_TYPE is in effect, we - can't create number objects in macros. */ -#ifndef make_number -Lisp_Object -make_number (EMACS_INT n) -{ - Lisp_Object obj; - obj.s.val = n; - obj.s.type = Lisp_Int; - return obj; -} -#endif /* Convert the pointer-sized word P to EMACS_INT while preserving its type and ptr fields. */ @@ -3156,7 +3142,7 @@ union aligned_Lisp_Symbol { struct Lisp_Symbol s; -#ifdef USE_LSB_TAG +#if USE_LSB_TAG unsigned char c[(sizeof (struct Lisp_Symbol) + (1 << GCTYPEBITS) - 1) & -(1 << GCTYPEBITS)]; #endif @@ -3262,7 +3248,7 @@ union aligned_Lisp_Misc { union Lisp_Misc m; -#ifdef USE_LSB_TAG +#if USE_LSB_TAG unsigned char c[(sizeof (union Lisp_Misc) + (1 << GCTYPEBITS) - 1) & -(1 << GCTYPEBITS)]; #endif @@ -4217,14 +4203,10 @@ { struct mem_node *m; - /* Quickly rule out some values which can't point to Lisp data. */ - if ((intptr_t) p % -#ifdef USE_LSB_TAG - 8 /* USE_LSB_TAG needs Lisp data to be aligned on multiples of 8. */ -#else - 2 /* We assume that Lisp data is aligned on even addresses. */ -#endif - ) + /* Quickly rule out some values which can't point to Lisp data. + USE_LSB_TAG needs Lisp data to be aligned on multiples of 8. + Otherwise, assume that Lisp data is aligned on even addresses. */ + if ((intptr_t) p % (USE_LSB_TAG ? 8 : 2)) return; m = mem_find (p); @@ -4299,8 +4281,8 @@ wider than a pointer might allocate a Lisp_Object in non-adjacent halves. If USE_LSB_TAG, the bottom half is not a valid pointer, but it should suffice to widen it to to a Lisp_Object and check it that way. */ -#if defined USE_LSB_TAG || VAL_MAX < UINTPTR_MAX -# if !defined USE_LSB_TAG && VAL_MAX < UINTPTR_MAX >> GCTYPEBITS +#if USE_LSB_TAG || VAL_MAX < UINTPTR_MAX +# if !USE_LSB_TAG && VAL_MAX < UINTPTR_MAX >> GCTYPEBITS /* If tag bits straddle pointer-word boundaries, neither mark_maybe_pointer nor mark_maybe_object can follow the pointers. This should not occur on any practical porting target. */ @@ -4728,7 +4710,7 @@ pure_alloc (size_t size, int type) { void *result; -#ifdef USE_LSB_TAG +#if USE_LSB_TAG size_t alignment = (1 << GCTYPEBITS); #else size_t alignment = sizeof (EMACS_INT); === modified file 'src/data.c' --- src/data.c 2012-05-30 03:59:42 +0000 +++ src/data.c 2012-06-01 23:34:07 +0000 @@ -1500,7 +1500,7 @@ { struct Lisp_Symbol *sym; struct Lisp_Buffer_Local_Value *blv = NULL; - union Lisp_Val_Fwd valcontents IF_LINT (= {0}); + union Lisp_Val_Fwd valcontents IF_LINT (= {LISP_INITIALLY_ZERO}); int forwarded IF_LINT (= 0); CHECK_SYMBOL (variable); @@ -1577,7 +1577,7 @@ { register Lisp_Object tem; int forwarded IF_LINT (= 0); - union Lisp_Val_Fwd valcontents IF_LINT (= {0}); + union Lisp_Val_Fwd valcontents IF_LINT (= {LISP_INITIALLY_ZERO}); struct Lisp_Symbol *sym; struct Lisp_Buffer_Local_Value *blv = NULL; === modified file 'src/emacs.c' --- src/emacs.c 2012-05-30 19:23:37 +0000 +++ src/emacs.c 2012-06-01 23:34:07 +0000 @@ -104,7 +104,7 @@ /* Make these values available in GDB, which doesn't see macros. */ -#ifdef USE_LSB_TAG +#if USE_LSB_TAG int gdb_use_lsb EXTERNALLY_VISIBLE = 1; #else int gdb_use_lsb EXTERNALLY_VISIBLE = 0; @@ -116,7 +116,7 @@ #endif int gdb_valbits EXTERNALLY_VISIBLE = VALBITS; int gdb_gctypebits EXTERNALLY_VISIBLE = GCTYPEBITS; -#if defined (DATA_SEG_BITS) && ! defined (USE_LSB_TAG) +#if defined DATA_SEG_BITS && !USE_LSB_TAG uintptr_t gdb_data_seg_bits EXTERNALLY_VISIBLE = DATA_SEG_BITS; #else uintptr_t gdb_data_seg_bits EXTERNALLY_VISIBLE = 0; === modified file 'src/frame.c' --- src/frame.c 2012-06-01 03:41:03 +0000 +++ src/frame.c 2012-06-01 23:34:07 +0000 @@ -1152,10 +1152,6 @@ described for Fdelete_frame. */ Lisp_Object delete_frame (Lisp_Object frame, Lisp_Object force) - /* If we use `register' here, gcc-4.0.2 on amd64 using - -DUSE_LISP_UNION_TYPE complains further down that we're getting the - address of `force'. Go figure. */ - { struct frame *f; struct frame *sf = SELECTED_FRAME (); === modified file 'src/ftfont.c' --- src/ftfont.c 2012-04-09 22:54:59 +0000 +++ src/ftfont.c 2012-06-01 23:34:07 +0000 @@ -525,7 +525,7 @@ struct font_driver ftfont_driver = { - 0, /* Qfreetype */ + LISP_INITIALLY_ZERO, /* Qfreetype */ 0, /* case insensitive */ ftfont_get_cache, ftfont_list, === modified file 'src/lisp.h' --- src/lisp.h 2012-05-30 19:23:37 +0000 +++ src/lisp.h 2012-06-01 23:34:07 +0000 @@ -161,10 +161,8 @@ always 0, and we can thus use them to hold tag bits, without restricting our addressing space. - If USE_LSB_TAG is not set, then we use the top 3 bits for tagging, thus - restricting our possible address range. Currently USE_LSB_TAG is not - allowed together with a union. This is not due to any fundamental - technical (or political ;-) problem: nobody wrote the code to do it yet. + If ! USE_LSB_TAG, then use the top 3 bits for tagging, thus + restricting our possible address range. USE_LSB_TAG not only requires the least 3 bits of pointers returned by malloc to be 0 but also needs to be able to impose a mult-of-8 alignment @@ -201,25 +199,31 @@ # endif #endif -/* Let's USE_LSB_TAG on systems where we know malloc returns mult-of-8. */ -#if (defined GNU_MALLOC || defined DOUG_LEA_MALLOC || defined __GLIBC__ \ - || defined DARWIN_OS || defined __sun) -/* We also need to be able to specify mult-of-8 alignment on static vars. */ -# if defined DECL_ALIGN -/* On hosts where pointers-as-ints do not exceed VAL_MAX, - USE_LSB_TAG is: +/* Unless otherwise specified, use USE_LSB_TAG on systems where: */ +#ifndef USE_LSB_TAG +/* 1. We know malloc returns a multiple of 8. */ +# if (defined GNU_MALLOC || defined DOUG_LEA_MALLOC || defined __GLIBC__ \ + || defined DARWIN_OS || defined __sun) +/* 2. We can specify multiple-of-8 alignment on static variables. */ +# ifdef DCL_ALIGN +/* 3. Pointers-as-ints exceed VAL_MAX. + On hosts where pointers-as-ints do not exceed VAL_MAX, USE_LSB_TAG is: a. unnecessary, because the top bits of an EMACS_INT are unused, and b. slower, because it typically requires extra masking. - So, define USE_LSB_TAG only on hosts where it might be useful. */ -# if VAL_MAX < UINTPTR_MAX -# define USE_LSB_TAG + So, default USE_LSB_TAG to 1 only on hosts where it might be useful. */ +# if VAL_MAX < UINTPTR_MAX +# define USE_LSB_TAG 1 +# endif # endif # endif #endif +#ifndef USE_LSB_TAG +# define USE_LSB_TAG 0 +#endif /* If we cannot use 8-byte alignment, make DECL_ALIGN a no-op. */ #ifndef DECL_ALIGN -# ifdef USE_LSB_TAG +# if USE_LSB_TAG # error "USE_LSB_TAG used without defining DECL_ALIGN" # endif # define DECL_ALIGN(type, var) type var @@ -248,7 +252,7 @@ #else # define LISP_INT_TAG Lisp_Int0 # define case_Lisp_Int case Lisp_Int0: case Lisp_Int1 -# ifdef USE_LSB_TAG +# if USE_LSB_TAG # define LISP_INT1_TAG 4 # define LISP_STRING_TAG 1 # define LISP_INT_TAG_P(x) (((x) & 3) == 0) @@ -333,70 +337,35 @@ #ifdef USE_LISP_UNION_TYPE -#ifndef WORDS_BIGENDIAN - -/* Definition of Lisp_Object for little-endian machines. */ - -typedef -union Lisp_Object - { - /* Used for comparing two Lisp_Objects; - also, positive integers can be accessed fast this way. */ - EMACS_INT i; - - struct - { - /* Use explicit signed, the signedness of a bit-field of type - int is implementation defined. */ - signed EMACS_INT val : VALBITS; - ENUM_BF (Lisp_Type) type : GCTYPEBITS; - } s; - struct - { - EMACS_UINT val : VALBITS; - ENUM_BF (Lisp_Type) type : GCTYPEBITS; - } u; - } -Lisp_Object; - -#else /* If WORDS_BIGENDIAN */ - -typedef -union Lisp_Object - { - /* Used for comparing two Lisp_Objects; - also, positive integers can be accessed fast this way. */ - EMACS_INT i; - - struct - { - ENUM_BF (Lisp_Type) type : GCTYPEBITS; - /* Use explicit signed, the signedness of a bit-field of type - int is implementation defined. */ - signed EMACS_INT val : VALBITS; - } s; - struct - { - ENUM_BF (Lisp_Type) type : GCTYPEBITS; - EMACS_UINT val : VALBITS; - } u; - } -Lisp_Object; - -#endif /* WORDS_BIGENDIAN */ - -#ifdef __GNUC__ +typedef +union Lisp_Object + { + /* Used for comparing two Lisp_Objects; + also, positive integers can be accessed fast this way. */ + EMACS_INT i; + + struct + { + /* Use explicit signed, the signedness of a bit-field of type + int is implementation defined. */ + signed EMACS_INT val : VALBITS; + ENUM_BF (Lisp_Type) type : GCTYPEBITS; + } s; + struct + { + EMACS_UINT val : VALBITS; + ENUM_BF (Lisp_Type) type : GCTYPEBITS; + } u; + } +Lisp_Object; + static inline Lisp_Object LISP_MAKE_RVALUE (Lisp_Object o) { return o; } -#else -/* This is more portable to pre-C99 non-GCC compilers, but for - backwards compatibility GCC still accepts an old GNU extension - which caused this to only generate a warning. */ -#define LISP_MAKE_RVALUE(o) (0 ? (o) : (o)) -#endif + +#define LISP_INITIALLY_ZERO {0} #else /* USE_LISP_UNION_TYPE */ @@ -404,6 +373,7 @@ typedef EMACS_INT Lisp_Object; #define LISP_MAKE_RVALUE(o) (0+(o)) +#define LISP_INITIALLY_ZERO 0 #endif /* USE_LISP_UNION_TYPE */ /* In the size word of a vector, this bit means the vector has been marked. */ @@ -467,7 +437,7 @@ /* Return a perfect hash of the Lisp_Object representation. */ #define XHASH(a) (a) -#ifdef USE_LSB_TAG +#if USE_LSB_TAG #define TYPEMASK ((((EMACS_INT) 1) << GCTYPEBITS) - 1) #define XTYPE(a) ((enum Lisp_Type) ((a) & TYPEMASK)) @@ -542,12 +512,12 @@ #define XINT(a) ((EMACS_INT) (a).s.val) #define XUINT(a) ((EMACS_UINT) (a).u.val) -#ifdef USE_LSB_TAG +#if USE_LSB_TAG # define XSET(var, vartype, ptr) \ - (eassert ((((uintptr_t) (ptr)) & ((1 << GCTYPEBITS) - 1)) == 0), \ - (var).u.val = ((uintptr_t) (ptr)) >> GCTYPEBITS, \ - (var).u.type = ((char) (vartype))) + (eassert (((uintptr_t) (ptr) & ((1 << GCTYPEBITS) - 1)) == 0), \ + (var).u.val = (uintptr_t) (ptr) >> GCTYPEBITS, \ + (var).u.type = (vartype)) /* Some versions of gcc seem to consider the bitfield width when issuing the "cast to pointer from integer of different size" warning, so the @@ -563,7 +533,7 @@ # define XSETFASTINT(a, b) ((a).i = (b)) # define XSET(var, vartype, ptr) \ - (((var).s.val = ((intptr_t) (ptr))), ((var).s.type = ((char) (vartype)))) + ((var).s.val = (intptr_t) (ptr), (var).s.type = (vartype)) #ifdef DATA_SEG_BITS /* DATA_SEG_BITS forces extra bits to be or'd in with any pointers @@ -575,12 +545,14 @@ #endif /* !USE_LSB_TAG */ -#if __GNUC__ >= 2 && defined (__OPTIMIZE__) -#define make_number(N) \ - (__extension__ ({ Lisp_Object _l; _l.s.val = (N); _l.s.type = Lisp_Int; _l; })) -#else -extern Lisp_Object make_number (EMACS_INT); -#endif +static inline Lisp_Object +make_number (EMACS_INT n) +{ + Lisp_Object o; + o.s.val = n; + o.s.type = Lisp_Int; + return o; +} #endif /* USE_LISP_UNION_TYPE */ === modified file 'src/mem-limits.h' --- src/mem-limits.h 2012-05-22 16:20:27 +0000 +++ src/mem-limits.h 2012-06-01 23:34:07 +0000 @@ -34,7 +34,7 @@ #endif extern char *start_of_data (void); -#if defined USE_LSB_TAG || UINTPTR_MAX <= VAL_MAX +#if USE_LSB_TAG || UINTPTR_MAX <= VAL_MAX #define EXCEEDS_LISP_PTR(ptr) 0 #elif defined DATA_SEG_BITS #define EXCEEDS_LISP_PTR(ptr) \ === modified file 'src/w32heap.c' --- src/w32heap.c 2012-03-25 18:30:50 +0000 +++ src/w32heap.c 2012-06-01 23:34:07 +0000 @@ -114,7 +114,7 @@ return data_region_end; } -#if !defined (USE_LISP_UNION_TYPE) && !defined (USE_LSB_TAG) +#if !USE_LSB_TAG static char * allocate_heap (void) { @@ -141,7 +141,7 @@ return ptr; } -#else /* USE_LISP_UNION_TYPE || USE_LSB_TAG */ +#else /* USE_LSB_TAG */ static char * allocate_heap (void) { @@ -160,7 +160,7 @@ return ptr; } -#endif /* USE_LISP_UNION_TYPE || USE_LSB_TAG */ +#endif /* USE_LSB_TAG */ /* Emulate Unix sbrk. Note that ralloc.c expects the return value to @@ -259,9 +259,9 @@ exit (1); } -#if !defined (USE_LISP_UNION_TYPE) && !defined (USE_LSB_TAG) +#if !USE_LSB_TAG /* Ensure that the addresses don't use the upper tag bits since - the Lisp type goes there. */ + Lisp_Object can't represent them. */ if (((unsigned long) data_region_base & ~VALMASK) != 0) { printf ("Error: The heap was allocated in upper memory.\n"); === modified file 'src/xfont.c' --- src/xfont.c 2012-02-10 18:58:48 +0000 +++ src/xfont.c 2012-06-01 23:34:07 +0000 @@ -132,7 +132,7 @@ struct font_driver xfont_driver = { - 0, /* Qx */ + LISP_INITIALLY_ZERO, /* Qx */ 0, /* case insensitive */ xfont_get_cache, xfont_list,
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.