From: Paul Eggert Date: Fri, 19 Oct 2018 16:06:52 +0000 (-0700) Subject: Fix struct thread alignment on FreeBSD x86 X-Git-Tag: emacs-27.0.90~4274 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=d2a07b9a82a632e8baa179c667a98d275e5f6973;p=emacs.git Fix struct thread alignment on FreeBSD x86 Problem reported by Joseph Mingrone in: https://lists.gnu.org/r/emacs-devel/2018-10/msg00238.html While we’re at it, apply a similar fix to struct Lisp_Subr; this removes the need for GCALIGNED_STRUCT_MEMBER and thus can shrink struct Lisp_Subr a bit. * configure.ac (HAVE_STRUCT_ATTRIBUTE_ALIGNED): Bring back this macro. Although used only for performance (not to actually align structures), we might as well take advantage of it. * src/lisp.h (GCALIGNED_STRUCT_MEMBER): Remove; all uses removed. (union Aligned_Lisp_Subr): New type, like struct Lisp_Subr but aligned. * src/lisp.h (XSUBR, DEFUN): * src/lread.c (defsubr): Use it. All callers changed. * src/thread.c (union aligned_thread_state): New type. (main_thread): Now of this type, so it’s aligned. All uses changed. * src/xmenu.c (syms_of_xmenu) [USE_GTK || USE_X_TOOLKIT]: Adjust to union Aligned_Lisp_Subr change. --- diff --git a/configure.ac b/configure.ac index 3a610909026..50e33335289 100644 --- a/configure.ac +++ b/configure.ac @@ -5207,6 +5207,22 @@ else fi AC_SUBST(LIBXMENU) +AC_CACHE_CHECK([for struct alignment], + [emacs_cv_struct_alignment], + [AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[#include + struct s { char c; } __attribute__ ((aligned (8))); + struct t { char c; struct s s; }; + char verify[offsetof (struct t, s) == 8 ? 1 : -1]; + ]])], + [emacs_cv_struct_alignment=yes], + [emacs_cv_struct_alignment=no])]) +if test "$emacs_cv_struct_alignment" = yes; then + AC_DEFINE([HAVE_STRUCT_ATTRIBUTE_ALIGNED], 1, + [Define to 1 if 'struct __attribute__ ((aligned (N)))' aligns the + structure to an N-byte boundary.]) +fi + if test "${GNU_MALLOC}" = "yes" ; then AC_DEFINE(GNU_MALLOC, 1, [Define to 1 if you want to use the GNU memory allocator.]) diff --git a/src/lisp.h b/src/lisp.h index 145901dff5e..eb6762678c7 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -229,7 +229,7 @@ extern bool suppress_checking EXTERNALLY_VISIBLE; 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 on some non-GC Lisp_Objects, all of which are aligned via - GCALIGNED_UNION_MEMBER, GCALIGNED_STRUCT_MEMBER, and GCALIGNED_STRUCT. */ + GCALIGNED_UNION_MEMBER. */ enum Lisp_Bits { @@ -284,15 +284,14 @@ error !; # define GCALIGNMENT 1 #endif -/* If a struct is always allocated by the GC and is therefore always - GC-aligned, put GCALIGNED_STRUCT after its closing '}'; this can - help the compiler generate better code. +/* To cause a union to have alignment of at least GCALIGNMENT, put + GCALIGNED_UNION_MEMBER in its member list. - To cause a union to have alignment of at least GCALIGNMENT, put - GCALIGNED_UNION_MEMBER in its member list. Similarly for a struct - and GCALIGNED_STRUCT_MEMBER, although this may make the struct a - bit bigger on non-GCC platforms. Any struct using - GCALIGNED_STRUCT_MEMBER should also use GCALIGNED_STRUCT. + If a struct is always GC-aligned (either by the GC, or via + allocation in a containing union that has GCALIGNED_UNION_MEMBER) + and does not contain a GC-aligned struct or union, putting + GCALIGNED_STRUCT after its closing '}' can help the compiler + generate better code. Although these macros are reasonably portable, they are not guaranteed on non-GCC platforms, as C11 does not require support @@ -306,10 +305,8 @@ error !; #define GCALIGNED_UNION_MEMBER char alignas (GCALIGNMENT) gcaligned; #if HAVE_STRUCT_ATTRIBUTE_ALIGNED -# define GCALIGNED_STRUCT_MEMBER # define GCALIGNED_STRUCT __attribute__ ((aligned (GCALIGNMENT))) #else -# define GCALIGNED_STRUCT_MEMBER GCALIGNED_UNION_MEMBER # define GCALIGNED_STRUCT #endif #define GCALIGNED(type) (alignof (type) % GCALIGNMENT == 0) @@ -1970,9 +1967,13 @@ struct Lisp_Subr const char *symbol_name; const char *intspec; EMACS_INT doc; - GCALIGNED_STRUCT_MEMBER } GCALIGNED_STRUCT; -verify (GCALIGNED (struct Lisp_Subr)); +union Aligned_Lisp_Subr + { + struct Lisp_Subr s; + GCALIGNED_UNION_MEMBER + }; +verify (GCALIGNED (union Aligned_Lisp_Subr)); INLINE bool SUBRP (Lisp_Object a) @@ -1984,7 +1985,7 @@ INLINE struct Lisp_Subr * XSUBR (Lisp_Object a) { eassert (SUBRP (a)); - return XUNTAG (a, Lisp_Vectorlike, struct Lisp_Subr); + return &XUNTAG (a, Lisp_Vectorlike, union Aligned_Lisp_Subr)->s; } enum char_table_specials @@ -2952,15 +2953,15 @@ CHECK_FIXNUM_CDR (Lisp_Object x) /* This version of DEFUN declares a function prototype with the right arguments, so we can catch errors with maxargs at compile-time. */ #define DEFUN(lname, fnname, sname, minargs, maxargs, intspec, doc) \ - static struct Lisp_Subr sname = \ - { { PVEC_SUBR << PSEUDOVECTOR_AREA_BITS }, \ + static union Aligned_Lisp_Subr sname = \ + {{{ PVEC_SUBR << PSEUDOVECTOR_AREA_BITS }, \ { .a ## maxargs = fnname }, \ - minargs, maxargs, lname, intspec, 0}; \ + minargs, maxargs, lname, intspec, 0}}; \ Lisp_Object fnname /* defsubr (Sname); is how we define the symbol for function `name' at start-up time. */ -extern void defsubr (struct Lisp_Subr *); +extern void defsubr (union Aligned_Lisp_Subr *); enum maxargs { diff --git a/src/lread.c b/src/lread.c index 62616cb6819..5f3871436df 100644 --- a/src/lread.c +++ b/src/lread.c @@ -4409,8 +4409,9 @@ init_obarray (void) } void -defsubr (struct Lisp_Subr *sname) +defsubr (union Aligned_Lisp_Subr *aname) { + struct Lisp_Subr *sname = &aname->s; Lisp_Object sym, tem; sym = intern_c_string (sname->symbol_name); XSETPVECTYPE (sname, PVEC_SUBR); diff --git a/src/thread.c b/src/thread.c index 3674af0e47b..6612697b95e 100644 --- a/src/thread.c +++ b/src/thread.c @@ -27,11 +27,18 @@ along with GNU Emacs. If not, see . */ #include "syssignal.h" #include "keyboard.h" -static struct thread_state main_thread; +union aligned_thread_state +{ + struct thread_state s; + GCALIGNED_UNION_MEMBER +}; +verify (GCALIGNED (union aligned_thread_state)); + +static union aligned_thread_state main_thread; -struct thread_state *current_thread = &main_thread; +struct thread_state *current_thread = &main_thread.s; -static struct thread_state *all_threads = &main_thread; +static struct thread_state *all_threads = &main_thread.s; static sys_mutex_t global_lock; @@ -113,7 +120,7 @@ maybe_reacquire_global_lock (void) /* SIGINT handler is always run on the main thread, see deliver_process_signal, so reflect that in our thread-tracking variables. */ - current_thread = &main_thread; + current_thread = &main_thread.s; if (current_thread->not_holding_lock) { @@ -659,7 +666,7 @@ mark_threads (void) void unmark_main_thread (void) { - main_thread.header.size &= ~ARRAY_MARK_FLAG; + main_thread.s.header.size &= ~ARRAY_MARK_FLAG; } @@ -1043,23 +1050,23 @@ thread_check_current_buffer (struct buffer *buffer) static void init_main_thread (void) { - main_thread.header.size + main_thread.s.header.size = PSEUDOVECSIZE (struct thread_state, m_stack_bottom); - XSETPVECTYPE (&main_thread, PVEC_THREAD); - main_thread.m_last_thing_searched = Qnil; - main_thread.m_saved_last_thing_searched = Qnil; - main_thread.name = Qnil; - main_thread.function = Qnil; - main_thread.result = Qnil; - main_thread.error_symbol = Qnil; - main_thread.error_data = Qnil; - main_thread.event_object = Qnil; + XSETPVECTYPE (&main_thread.s, PVEC_THREAD); + main_thread.s.m_last_thing_searched = Qnil; + main_thread.s.m_saved_last_thing_searched = Qnil; + main_thread.s.name = Qnil; + main_thread.s.function = Qnil; + main_thread.s.result = Qnil; + main_thread.s.error_symbol = Qnil; + main_thread.s.error_data = Qnil; + main_thread.s.event_object = Qnil; } bool main_thread_p (void *ptr) { - return ptr == &main_thread; + return ptr == &main_thread.s; } bool @@ -1080,11 +1087,11 @@ void init_threads (void) { init_main_thread (); - sys_cond_init (&main_thread.thread_condvar); + sys_cond_init (&main_thread.s.thread_condvar); sys_mutex_init (&global_lock); sys_mutex_lock (&global_lock); - current_thread = &main_thread; - main_thread.thread_id = sys_thread_self (); + current_thread = &main_thread.s; + main_thread.s.thread_id = sys_thread_self (); } void @@ -1130,7 +1137,7 @@ syms_of_threads (void) DEFVAR_LISP ("main-thread", Vmain_thread, doc: /* The main thread of Emacs. */); #ifdef THREADS_ENABLED - XSETTHREAD (Vmain_thread, &main_thread); + XSETTHREAD (Vmain_thread, &main_thread.s); #else Vmain_thread = Qnil; #endif diff --git a/src/xmenu.c b/src/xmenu.c index 10e882af439..31034f71122 100644 --- a/src/xmenu.c +++ b/src/xmenu.c @@ -2420,6 +2420,6 @@ syms_of_xmenu (void) #if defined (USE_GTK) || defined (USE_X_TOOLKIT) defsubr (&Sx_menu_bar_open_internal); Ffset (intern_c_string ("accelerate-menu"), - intern_c_string (Sx_menu_bar_open_internal.symbol_name)); + intern_c_string (Sx_menu_bar_open_internal.s.symbol_name)); #endif }