GNU bug report logs - #11604
USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.

Previous Next

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.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11604 in the body.
You can then email your comments to 11604 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Fri, 01 Jun 2012 23:40:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Eggert <eggert <at> cs.ucla.edu>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 01 Jun 2012 23:40:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

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 -0700
Tags: 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,





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sat, 02 Jun 2012 07:34:01 GMT) Full text and rfc822 format available.

Message #8 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sat, 02 Jun 2012 10:31:19 +0300
> Date: Fri, 01 Jun 2012 16:37:15 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> 
> Tags: 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.

Could you please explain this part:

> +	* w32heap.c (allocate_heap, init_heap):
> +	Pay no attention to USE_LISP_UNION_TYPE, as it's irrelevant here.

In what way is it "irrelevant here"?  Your changes don't remove
USE_LISP_UNION_TYPE, do they?  And using the union still limits the
address space, doesn't it?

Also, why did you change DECL_ALIGN to DCL_ALIGN? was something wrong
with the previous name?  The log entry doesn't mention this change,
and the rest of lisp.h that uses DECL_ALIGN was not modified.  Will
this even compile?

Re this:

> +	(union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the
> +	code works fine either way, and efficiency is not a concern here.

why isn't efficiency a concern?

Re this:

> +	(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.

I still see one instance of LISP_MAKE_RVALUE as a macro in lisp.h:

> @@ -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 */

Also, which parts of gnulib make 'static inline' portable?  If it
isn't in a part that is used by the MS-Windows port, the MSVC build is
still being screwed by these changes.

The following changes are not mentioned in the log entry:

>  # 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))

What was the reason for removing this comment:

> --- 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 ();

?  It is again not mentioned in the log entry.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sat, 02 Jun 2012 07:56:02 GMT) Full text and rfc822 format available.

Message #11 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sat, 02 Jun 2012 09:52:47 +0200
Paul Eggert <eggert <at> cs.ucla.edu> writes:

> +	(union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the
> +	code works fine either way,

Huh?  Of course it doesn't.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 06:20:01 GMT) Full text and rfc822 format available.

Message #14 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sat, 02 Jun 2012 23:17:38 -0700
On 06/02/2012 12:52 AM, Andreas Schwab wrote:
> Huh?  Of course it doesn't.

Thanks for catching that.  I'll supply a revised fix that
omits the definitions of XFASTINT and XSETFASTINT when
USE_LISP_UNION_TYPE, which should fix the bug you mention,
and which further simplifies the source code.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 06:53:02 GMT) Full text and rfc822 format available.

Message #17 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sat, 02 Jun 2012 23:50:07 -0700
On 06/02/2012 12:31 AM, Eli Zaretskii wrote:
> why did you change DECL_ALIGN to DCL_ALIGN?

A typo.  It didn't prevent compilation on my platform.  Thanks for
pointing it out; the revised patch will fix this.

>> +	(union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the
>> +	code works fine either way, and efficiency is not a concern here.
>
> why isn't efficiency a concern?

Because the union type is for debugging, not for production.
It's used mostly to make sure that (for example) one doesn't
mistakenly write "x = y" when one meant to write "x = XINT (y)".
I'll reword the ChangeLog entry to make it clearer.

>> +	(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.
>
> I still see one instance of LISP_MAKE_RVALUE as a macro in lisp.h:

The ChangeLog entry is only about the USE_LISP_UNION_TYPE case, not
the case you noticed.  I'll reword this for clarity.

> Also, which parts of gnulib make 'static inline' portable?  If it
> isn't in a part that is used by the MS-Windows port, the MSVC build is
> still being screwed by these changes.

Mostly, it's the AC_C_INLINE stuff in m4/*.  'static inline' been used
for some time in Emacs, and it works just fine with the MSVC build.
I'll reword the ChangeLog entry to remove the distracting mention of
gnulib.

> The following changes are not mentioned in the log entry:

They are mentioned here:

	(XSET) [USE_LISP_UNION_TYPE]: Don't overparenthesize.

> What was the reason for removing this comment:

Please see Bug#11617, which I just now filed.  I will restore that
comment in the revised patch, as it's now an independent issue.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 06:57:02 GMT) Full text and rfc822 format available.

Message #20 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 11604 <at> debbugs.gnu.org
Subject: revised patch for Bug #11604
Date: Sat, 02 Jun 2012 23:54:35 -0700
Here is a revised patch for Bug#11604.
It attempts to address all the comments raised.
Thanks to everybody who reviewed the earlier patch.

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-06-02 21:01:07 +0000
+++ src/ChangeLog	2012-06-03 06:39:29 +0000
@@ -1,3 +1,26 @@
+2012-06-03  Paul Eggert  <eggert <at> cs.ucla.edu>
+
+	USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup (Bug#11604)
+	* 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,
+	as the union type is for debugging, not for production.
+	(LISP_MAKE_RVALUE, make_number) [USE_LISP_UNION_TYPE]:
+	Use an inline function on all platforms when using the union type,
+	since this is simpler and 'static inline' can be used portably
+	within Emacs now.
+	(LISP_INITIALLY_ZERO): New macro.
+	(XFASTINT, XSETFASTINT) [USE_LISP_UNION_TYPE]: Remove.
+	(XSET) [USE_LISP_UNION_TYPE]: Don't overparenthesize.
+
 2012-06-02  Paul Eggert  <eggert <at> cs.ucla.edu>
 
 	* sysdep.c (system_process_attributes): Improve comment.

=== modified file 'src/alloc.c'
--- src/alloc.c	2012-06-02 08:52:27 +0000
+++ src/alloc.c	2012-06-02 15:19:06 +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.  */
@@ -3158,7 +3144,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
@@ -3264,7 +3250,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
@@ -4219,14 +4205,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);
@@ -4301,8 +4283,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.  */
@@ -4730,7 +4712,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-03 06:39:29 +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 DECL_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
@@ -556,14 +526,8 @@
 
 #else  /* !USE_LSB_TAG */
 
-/* For integers known to be positive, XFASTINT provides fast retrieval
-   and XSETFASTINT provides fast storage.  This takes advantage of the
-   fact that Lisp_Int is 0.  */
-# define XFASTINT(a) ((a).i + 0)
-# 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 +539,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/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,




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 07:30:02 GMT) Full text and rfc822 format available.

Message #23 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sun, 03 Jun 2012 09:26:46 +0200
Paul Eggert <eggert <at> cs.ucla.edu> writes:

> Because the union type is for debugging, not for production.
> It's used mostly to make sure that (for example) one doesn't
> mistakenly write "x = y" when one meant to write "x = XINT (y)".

That doesn't make sense.  You don't need a bitfield for that, just a
struct around EMACS_INT.  If we want to go that route we should rip out
the union Lisp_Object completely.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 07:40:01 GMT) Full text and rfc822 format available.

Message #26 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sun, 03 Jun 2012 00:37:22 -0700
On 06/03/2012 12:26 AM, Andreas Schwab wrote:
> You don't need a bitfield for that, just a
> struct around EMACS_INT.

That thought had also occurred to me.  I avoided it only
because it would be a bigger change.  But it would also
work and it would clean things up further.

My impression is that the union + bitfield was the original implementation,
that it was replaced long ago by integers for performance reasons, and that
the only remaining reason for the union is for debugging.
For example, "./configure --help" says:

  --enable-use-lisp-union-type
                          use a union for the Lisp_Object data type. This is
                          only useful for development for catching certain
                          types of bugs.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 14:34:02 GMT) Full text and rfc822 format available.

Message #29 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sun, 03 Jun 2012 17:31:18 +0300
> Date: Sat, 02 Jun 2012 23:50:07 -0700
> From: Paul Eggert <eggert <at> cs.ucla.edu>
> CC: 11604 <at> debbugs.gnu.org
> 
> >> +	(union Lisp_Object): Don't worry about WORDS_BIGENDIAN; the
> >> +	code works fine either way, and efficiency is not a concern here.
> >
> > why isn't efficiency a concern?
> 
> Because the union type is for debugging, not for production.

I didn't know that.  If that is so, it should be mentioned in the
comments somewhere.  (I actually thought that Stefan wants to move to
that model for production at some point, but maybe I misunderstood.)

> > Also, which parts of gnulib make 'static inline' portable?  If it
> > isn't in a part that is used by the MS-Windows port, the MSVC build is
> > still being screwed by these changes.
> 
> Mostly, it's the AC_C_INLINE stuff in m4/*.

Yes, but where in the gnulib stuff that is compiled does it make
'static inline' portable?  The m4/* files aren't used for compiling
and linking, so that's not the answer I was looking for.

> 'static inline' been used for some time in Emacs, and it works just
> fine with the MSVC build.

I believe you, but I'd like still to make sure I understand all the
aspects of this.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 16:43:01 GMT) Full text and rfc822 format available.

Message #32 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sun, 03 Jun 2012 09:40:23 -0700
On 06/03/2012 07:31 AM, Eli Zaretskii wrote:
> where in the gnulib stuff that is compiled does it make
> 'static inline' portable?  The m4/* files aren't used for compiling
> and linking

The m4/* files control whether src/config.h contains
'#define inline /* whatever */', and this is what ports
'static inline' to compilers that spell 'inline' in some
other way (e.g., '__inline'), or which don't support
'inline' at all.

MSVC doesn't need to worry about this, as it supports
'static inline'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Sun, 03 Jun 2012 18:33:02 GMT) Full text and rfc822 format available.

Message #35 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 11604 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Sun, 03 Jun 2012 14:30:54 -0400
If using the union type is indeed less efficient, maybe that means
improvements in GCC are called for.

--
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Mon, 04 Jun 2012 13:15:02 GMT) Full text and rfc822 format available.

Message #38 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 11604 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Mon, 04 Jun 2012 09:12:23 -0400
>> Because the union type is for debugging, not for production.
> I didn't know that.  If that is so, it should be mentioned in the
> comments somewhere.  (I actually thought that Stefan wants to move to
> that model for production at some point, but maybe I misunderstood.)

I'm not sure what it is you misunderstood, but I don't intend to use
this "union type for Lisp_Object" for production.  I encourage people
around here to use it, to catch errors a bit earlier (tho, since they're
caught by the compiler, a single user such as myself is enough to get
a pretty good code coverage), but it has too many downsides
(performance is one, 29bit ints vs 30bit ints is another).
I would be OK with replacing it with a "struct".


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Mon, 04 Jun 2012 13:41:02 GMT) Full text and rfc822 format available.

Message #41 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 11604 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Mon, 04 Jun 2012 16:38:38 +0300
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Paul Eggert <eggert <at> cs.ucla.edu>,  11604 <at> debbugs.gnu.org
> Date: Mon, 04 Jun 2012 09:12:23 -0400
> 
> >> Because the union type is for debugging, not for production.
> > I didn't know that.  If that is so, it should be mentioned in the
> > comments somewhere.  (I actually thought that Stefan wants to move to
> > that model for production at some point, but maybe I misunderstood.)
> 
> I'm not sure what it is you misunderstood, but I don't intend to use
> this "union type for Lisp_Object" for production.  I encourage
> people around here to use it, to catch errors a bit earlier

My misunderstanding, sorry.  I probably misinterpreted your
encouragements.

> such as myself is enough to get a pretty good code coverage), but it
> has too many downsides

OK, then I think we should just say in the comments that
USE_LISP_UNION_TYPE is for debugging and maintenance, and is not meant
to yield production grade code.

> (performance is one, 29bit ints vs 30bit ints is another)

If using USE_LISP_UNION_TYPE means EMACS_INT is one bit less wide,
then I guess w32heap.c is correct in testing both USE_LSB_TAG and
USE_LISP_UNION_TYPE, when it decides how much VM to reserve at
startup?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Mon, 04 Jun 2012 13:53:02 GMT) Full text and rfc822 format available.

Message #44 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 11604 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#11604: USE_LISP_UNION_TYPE + USE_LSB_TAG cleanup.
Date: Mon, 04 Jun 2012 06:49:40 -0700
On 06/04/2012 06:38 AM, Eli Zaretskii wrote:
>> > (performance is one, 29bit ints vs 30bit ints is another)
> If using USE_LISP_UNION_TYPE means EMACS_INT is one bit less wide,
> then I guess w32heap.c is correct in testing both USE_LSB_TAG and
> USE_LISP_UNION_TYPE, when it decides how much VM to reserve at
> startup?

No, the two issues are independent.  The USE_LISP_UNION_TYPE
business in w32heap.c is about the width of pointers that
can be stuffed into Lisp_Object values; on hosts with 32-bit
EMACS_INT this width is 29 bits regardless of whether
USE_LISP_UNION_TYPE is set.  In other words, Stefan's comments
were about FIXNUM_BITS, not VALBITS.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Wed, 13 Jun 2012 10:15:02 GMT) Full text and rfc822 format available.

Message #47 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: revised patch for Bug #11604
Date: Wed, 13 Jun 2012 12:11:40 +0200
Paul Eggert <eggert <at> cs.ucla.edu> writes:

> +typedef
> +union Lisp_Object
> +  {
> +    /* Used for comparing two Lisp_Objects;
> +       also, positive integers can be accessed fast this way.  */

The comment doesn't make any sense any more.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#11604; Package emacs. (Thu, 14 Jun 2012 02:34:02 GMT) Full text and rfc822 format available.

Message #50 received at 11604 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 11604 <at> debbugs.gnu.org
Subject: Re: bug#11604: revised patch for Bug #11604
Date: Wed, 13 Jun 2012 19:30:48 -0700
On 06/13/2012 03:11 AM, Andreas Schwab wrote:
> The comment doesn't make any sense any more.

Thanks for fixing that in trunk bzr 108598.
Wow, nice fix!  I tagged along and tried to clean up
lisp.h a bit more, as trunk bzr 108606.




Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Sun, 29 Jul 2012 07:37:01 GMT) Full text and rfc822 format available.

Notification sent to Paul Eggert <eggert <at> cs.ucla.edu>:
bug acknowledged by developer. (Sun, 29 Jul 2012 07:37:01 GMT) Full text and rfc822 format available.

Message #55 received at 11604-done <at> debbugs.gnu.org (full text, mbox):

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 -0700
In 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.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 26 Aug 2012 11:24:02 GMT) Full text and rfc822 format available.

This bug report was last modified 11 years and 218 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.