>From b6f194a0fb6dbd1b19aa01f95a955f5b8b23b40e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 20 Jul 2019 19:40:03 -0700 Subject: [PATCH 5/6] Simplify hashfn/cmpfn calling convention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/fns.c (cmpfn_eql, cmpfn_equal, cmpfn_user_defined) (hashfn_eq, hashfn_equal, hashfn_eql, hashfn_user_defined): * src/profiler.c (cmpfn_profiler, hashfn_profiler): Use new calling convention where the return value is a fixnum instead of EMACS_UINT. While we’re at it, put the hash table at the end, since that’s a bit simpler and generates better code (at least on the x86-64). All callers changed. * src/fns.c (hash_lookup): Store fixnum rather than EMACS_UINT. All callers changed. (hash_put): Take a fixnum rather than an EMACS_UINT. All callers changed. Remove unnecessary eassert (XUFIXNUM does it). * src/lisp.h (struct hash_table_test): Adjust signatures of cmpfn and hashfn. --- src/bytecode.c | 8 ++-- src/category.c | 9 ++-- src/charset.c | 2 +- src/composite.c | 5 +-- src/emacs-module.c | 3 +- src/fns.c | 103 ++++++++++++++++++++------------------------- src/image.c | 3 +- src/json.c | 3 +- src/lisp.h | 12 +++--- src/lread.c | 3 +- src/macfont.m | 3 +- src/profiler.c | 34 +++++++-------- 12 files changed, 82 insertions(+), 106 deletions(-) diff --git a/src/bytecode.c b/src/bytecode.c index 29dff44f00..e82de026a8 100644 --- a/src/bytecode.c +++ b/src/bytecode.c @@ -1409,16 +1409,16 @@ #define DEFINE(name, value) LABEL (name) , if (h->count <= 5) { /* Do a linear search if there are not many cases FIXME: 5 is arbitrarily chosen. */ - Lisp_Object hash_code = h->test.cmpfn - ? make_fixnum (h->test.hashfn (&h->test, v1)) : Qnil; + Lisp_Object hash_code + = h->test.cmpfn ? h->test.hashfn (v1, &h->test) : Qnil; for (i = h->count; 0 <= --i; ) if (EQ (v1, HASH_KEY (h, i)) || (h->test.cmpfn && EQ (hash_code, HASH_HASH (h, i)) - && h->test.cmpfn (&h->test, v1, HASH_KEY (h, i)))) + && !NILP (h->test.cmpfn (v1, HASH_KEY (h, i), + &h->test)))) break; - } else i = hash_lookup (h, v1, NULL); diff --git a/src/category.c b/src/category.c index 132fae9d40..9e460cfc64 100644 --- a/src/category.c +++ b/src/category.c @@ -48,18 +48,15 @@ bset_category_table (struct buffer *b, Lisp_Object val) static Lisp_Object hash_get_category_set (Lisp_Object table, Lisp_Object category_set) { - struct Lisp_Hash_Table *h; - ptrdiff_t i; - EMACS_UINT hash; - if (NILP (XCHAR_TABLE (table)->extras[1])) set_char_table_extras (table, 1, make_hash_table (hashtest_equal, DEFAULT_HASH_SIZE, DEFAULT_REHASH_SIZE, DEFAULT_REHASH_THRESHOLD, Qnil, false)); - h = XHASH_TABLE (XCHAR_TABLE (table)->extras[1]); - i = hash_lookup (h, category_set, &hash); + struct Lisp_Hash_Table *h = XHASH_TABLE (XCHAR_TABLE (table)->extras[1]); + Lisp_Object hash; + ptrdiff_t i = hash_lookup (h, category_set, &hash); if (i >= 0) return HASH_KEY (h, i); hash_put (h, category_set, Qnil, hash); diff --git a/src/charset.c b/src/charset.c index 85535e8bff..8c54381dc4 100644 --- a/src/charset.c +++ b/src/charset.c @@ -842,7 +842,7 @@ DEFUN ("define-charset-internal", Fdefine_charset_internal, /* Charset attr vector. */ Lisp_Object attrs; Lisp_Object val; - EMACS_UINT hash_code; + Lisp_Object hash_code; struct Lisp_Hash_Table *hash_table = XHASH_TABLE (Vcharset_hash_table); int i, j; struct charset charset; diff --git a/src/composite.c b/src/composite.c index 183062de46..c36663f8e9 100644 --- a/src/composite.c +++ b/src/composite.c @@ -164,11 +164,10 @@ #define MAX_AUTO_COMPOSITION_LOOKBACK 3 get_composition_id (ptrdiff_t charpos, ptrdiff_t bytepos, ptrdiff_t nchars, Lisp_Object prop, Lisp_Object string) { - Lisp_Object id, length, components, key, *key_contents; + Lisp_Object id, length, components, key, *key_contents, hash_code; ptrdiff_t glyph_len; struct Lisp_Hash_Table *hash_table = XHASH_TABLE (composition_hash_table); ptrdiff_t hash_index; - EMACS_UINT hash_code; enum composition_method method; struct composition *cmp; ptrdiff_t i; @@ -656,7 +655,7 @@ composition_gstring_put_cache (Lisp_Object gstring, ptrdiff_t len) struct Lisp_Hash_Table *h = XHASH_TABLE (gstring_hash_table); hash_rehash_if_needed (h); Lisp_Object header = LGSTRING_HEADER (gstring); - EMACS_UINT hash = h->test.hashfn (&h->test, header); + Lisp_Object hash = h->test.hashfn (header, &h->test); if (len < 0) { ptrdiff_t glyph_len = LGSTRING_GLYPH_LEN (gstring); diff --git a/src/emacs-module.c b/src/emacs-module.c index 8c09ea6bb6..4b991a1c74 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -362,8 +362,7 @@ module_make_global_ref (emacs_env *env, emacs_value ref) { MODULE_FUNCTION_BEGIN (NULL); struct Lisp_Hash_Table *h = XHASH_TABLE (Vmodule_refs_hash); - Lisp_Object new_obj = value_to_lisp (ref); - EMACS_UINT hashcode; + Lisp_Object new_obj = value_to_lisp (ref), hashcode; ptrdiff_t i = hash_lookup (h, new_obj, &hashcode); if (i >= 0) diff --git a/src/fns.c b/src/fns.c index d4f6842f27..d9503c491e 100644 --- a/src/fns.c +++ b/src/fns.c @@ -2373,7 +2373,7 @@ internal_equal (Lisp_Object o1, Lisp_Object o2, enum equal_kind equal_kind, case Lisp_Cons: case Lisp_Vectorlike: { struct Lisp_Hash_Table *h = XHASH_TABLE (ht); - EMACS_UINT hash; + Lisp_Object hash; ptrdiff_t i = hash_lookup (h, o1, &hash); if (i >= 0) { /* `o1' was seen already. */ @@ -3934,74 +3934,67 @@ HASH_INDEX (struct Lisp_Hash_Table *h, ptrdiff_t idx) /* Ignore HT and compare KEY1 and KEY2 using 'eql'. Value is true if KEY1 and KEY2 are the same. */ -static bool -cmpfn_eql (struct hash_table_test *ht, - Lisp_Object key1, - Lisp_Object key2) +static Lisp_Object +cmpfn_eql (Lisp_Object key1, Lisp_Object key2, struct hash_table_test *ht) { - return !NILP (Feql (key1, key2)); + return Feql (key1, key2); } /* Ignore HT and compare KEY1 and KEY2 using 'equal'. Value is true if KEY1 and KEY2 are the same. */ -static bool -cmpfn_equal (struct hash_table_test *ht, - Lisp_Object key1, - Lisp_Object key2) +static Lisp_Object +cmpfn_equal (Lisp_Object key1, Lisp_Object key2, struct hash_table_test *ht) { - return !NILP (Fequal (key1, key2)); + return Fequal (key1, key2); } /* Given HT, compare KEY1 and KEY2 using HT->user_cmp_function. Value is true if KEY1 and KEY2 are the same. */ -static bool -cmpfn_user_defined (struct hash_table_test *ht, - Lisp_Object key1, - Lisp_Object key2) +static Lisp_Object +cmpfn_user_defined (Lisp_Object key1, Lisp_Object key2, + struct hash_table_test *ht) { - return !NILP (call2 (ht->user_cmp_function, key1, key2)); + return call2 (ht->user_cmp_function, key1, key2); } -/* Ignore HT and return a hash code for KEY which uses 'eq' to compare keys. - The hash code is at most INTMASK. */ +/* Ignore HT and return a hash code for KEY which uses 'eq' to compare + keys. */ -static EMACS_UINT -hashfn_eq (struct hash_table_test *ht, Lisp_Object key) +static Lisp_Object +hashfn_eq (Lisp_Object key, struct hash_table_test *ht) { - return XHASH (key) ^ XTYPE (key); + return make_fixnum (XHASH (key) ^ XTYPE (key)); } /* Ignore HT and return a hash code for KEY which uses 'equal' to compare keys. The hash code is at most INTMASK. */ -EMACS_UINT -hashfn_equal (struct hash_table_test *ht, Lisp_Object key) +Lisp_Object +hashfn_equal (Lisp_Object key, struct hash_table_test *ht) { - return sxhash (key, 0); + return make_fixnum (sxhash (key, 0)); } /* Ignore HT and return a hash code for KEY which uses 'eql' to compare keys. The hash code is at most INTMASK. */ -EMACS_UINT -hashfn_eql (struct hash_table_test *ht, Lisp_Object key) +Lisp_Object +hashfn_eql (Lisp_Object key, struct hash_table_test *ht) { - return ((FLOATP (key) || BIGNUMP (key)) - ? hashfn_equal (ht, key) - : hashfn_eq (ht, key)); + return (FLOATP (key) || BIGNUMP (key) ? hashfn_equal : hashfn_eq) (key, ht); } /* Given HT, return a hash code for KEY which uses a user-defined - function to compare keys. The hash code is at most INTMASK. */ + function to compare keys. */ -static EMACS_UINT -hashfn_user_defined (struct hash_table_test *ht, Lisp_Object key) +static Lisp_Object +hashfn_user_defined (Lisp_Object key, struct hash_table_test *ht) { Lisp_Object hash = call1 (ht->user_hash_function, key); - return hashfn_eq (ht, hash); + return hashfn_eq (hash, ht); } struct hash_table_test const @@ -4224,8 +4217,8 @@ hash_table_rehash (struct Lisp_Hash_Table *h) if (!NILP (HASH_HASH (h, i))) { Lisp_Object key = HASH_KEY (h, i); - EMACS_UINT hash_code = h->test.hashfn (&h->test, key); - set_hash_hash_slot (h, i, make_fixnum (hash_code)); + Lisp_Object hash_code = h->test.hashfn (key, &h->test); + set_hash_hash_slot (h, i, hash_code); } /* Reset the index so that any slot we don't fill below is marked @@ -4256,25 +4249,23 @@ hash_table_rehash (struct Lisp_Hash_Table *h) matching KEY, or -1 if not found. */ ptrdiff_t -hash_lookup (struct Lisp_Hash_Table *h, Lisp_Object key, EMACS_UINT *hash) +hash_lookup (struct Lisp_Hash_Table *h, Lisp_Object key, Lisp_Object *hash) { - EMACS_UINT hash_code; ptrdiff_t start_of_bucket, i; hash_rehash_if_needed (h); - hash_code = h->test.hashfn (&h->test, key); - eassert ((hash_code & ~INTMASK) == 0); + Lisp_Object hash_code = h->test.hashfn (key, &h->test); if (hash) *hash = hash_code; - start_of_bucket = hash_code % ASIZE (h->index); + start_of_bucket = XUFIXNUM (hash_code) % ASIZE (h->index); for (i = HASH_INDEX (h, start_of_bucket); 0 <= i; i = HASH_NEXT (h, i)) if (EQ (key, HASH_KEY (h, i)) || (h->test.cmpfn - && hash_code == XUFIXNUM (HASH_HASH (h, i)) - && h->test.cmpfn (&h->test, key, HASH_KEY (h, i)))) + && EQ (hash_code, HASH_HASH (h, i)) + && !NILP (h->test.cmpfn (key, HASH_KEY (h, i), &h->test)))) break; return i; @@ -4287,14 +4278,12 @@ hash_lookup (struct Lisp_Hash_Table *h, Lisp_Object key, EMACS_UINT *hash) ptrdiff_t hash_put (struct Lisp_Hash_Table *h, Lisp_Object key, Lisp_Object value, - EMACS_UINT hash) + Lisp_Object hash) { ptrdiff_t start_of_bucket, i; hash_rehash_if_needed (h); - eassert ((hash & ~INTMASK) == 0); - /* Increment count after resizing because resizing may fail. */ maybe_resize_hash_table (h); h->count++; @@ -4306,10 +4295,10 @@ hash_put (struct Lisp_Hash_Table *h, Lisp_Object key, Lisp_Object value, set_hash_value_slot (h, i, value); /* Remember its hash code. */ - set_hash_hash_slot (h, i, make_fixnum (hash)); + set_hash_hash_slot (h, i, hash); /* Add new entry to its collision chain. */ - start_of_bucket = hash % ASIZE (h->index); + start_of_bucket = XUFIXNUM (hash) % ASIZE (h->index); set_hash_next_slot (h, i, HASH_INDEX (h, start_of_bucket)); set_hash_index_slot (h, start_of_bucket, i); return i; @@ -4321,9 +4310,8 @@ hash_put (struct Lisp_Hash_Table *h, Lisp_Object key, Lisp_Object value, void hash_remove_from_table (struct Lisp_Hash_Table *h, Lisp_Object key) { - EMACS_UINT hash_code = h->test.hashfn (&h->test, key); - eassert ((hash_code & ~INTMASK) == 0); - ptrdiff_t start_of_bucket = hash_code % ASIZE (h->index); + Lisp_Object hash_code = h->test.hashfn (key, &h->test); + ptrdiff_t start_of_bucket = XUFIXNUM (hash_code) % ASIZE (h->index); ptrdiff_t prev = -1; hash_rehash_if_needed (h); @@ -4334,8 +4322,8 @@ hash_remove_from_table (struct Lisp_Hash_Table *h, Lisp_Object key) { if (EQ (key, HASH_KEY (h, i)) || (h->test.cmpfn - && hash_code == XUFIXNUM (HASH_HASH (h, i)) - && h->test.cmpfn (&h->test, key, HASH_KEY (h, i)))) + && EQ (hash_code, HASH_HASH (h, i)) + && !NILP (h->test.cmpfn (key, HASH_KEY (h, i), &h->test)))) { /* Take entry out of collision chain. */ if (prev < 0) @@ -4685,7 +4673,7 @@ If (eq A B), then (= (sxhash-eq A) (sxhash-eq B)). Hash codes are not guaranteed to be preserved across Emacs sessions. */) (Lisp_Object obj) { - return make_fixnum (hashfn_eq (NULL, obj)); + return hashfn_eq (obj, NULL); } DEFUN ("sxhash-eql", Fsxhash_eql, Ssxhash_eql, 1, 1, 0, @@ -4695,7 +4683,7 @@ If (eql A B), then (= (sxhash-eql A) (sxhash-eql B)). Hash codes are not guaranteed to be preserved across Emacs sessions. */) (Lisp_Object obj) { - return make_fixnum (hashfn_eql (NULL, obj)); + return hashfn_eql (obj, NULL); } DEFUN ("sxhash-equal", Fsxhash_equal, Ssxhash_equal, 1, 1, 0, @@ -4705,7 +4693,7 @@ If (equal A B), then (= (sxhash-equal A) (sxhash-equal B)). Hash codes are not guaranteed to be preserved across Emacs sessions. */) (Lisp_Object obj) { - return make_fixnum (hashfn_equal (NULL, obj)); + return hashfn_equal (obj, NULL); } DEFUN ("make-hash-table", Fmake_hash_table, Smake_hash_table, 0, MANY, 0, @@ -4951,9 +4939,8 @@ DEFUN ("puthash", Fputhash, Sputhash, 3, 3, 0, struct Lisp_Hash_Table *h = check_hash_table (table); CHECK_IMPURE (table, h); - ptrdiff_t i; - EMACS_UINT hash; - i = hash_lookup (h, key, &hash); + Lisp_Object hash; + ptrdiff_t i = hash_lookup (h, key, &hash); if (i >= 0) set_hash_value_slot (h, i, value); else diff --git a/src/image.c b/src/image.c index b081d4b912..355c849491 100644 --- a/src/image.c +++ b/src/image.c @@ -4606,8 +4606,7 @@ xpm_put_color_table_h (Lisp_Object color_table, Lisp_Object color) { struct Lisp_Hash_Table *table = XHASH_TABLE (color_table); - EMACS_UINT hash_code; - Lisp_Object chars = make_unibyte_string (chars_start, chars_len); + Lisp_Object chars = make_unibyte_string (chars_start, chars_len), hash_code; hash_lookup (table, chars, &hash_code); hash_put (table, chars, color, hash_code); diff --git a/src/json.c b/src/json.c index 21c4b946b4..d05f2c54e2 100644 --- a/src/json.c +++ b/src/json.c @@ -867,8 +867,7 @@ json_to_lisp (json_t *json, struct json_configuration *conf) json_t *value; json_object_foreach (json, key_str, value) { - Lisp_Object key = build_string_from_utf8 (key_str); - EMACS_UINT hash; + Lisp_Object key = build_string_from_utf8 (key_str), hash; ptrdiff_t i = hash_lookup (h, key, &hash); /* Keys in JSON objects are unique, so the key can't be present yet. */ diff --git a/src/lisp.h b/src/lisp.h index 50a61cadd7..e5edb8fd12 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -2237,10 +2237,10 @@ #define DEFSYM(sym, name) /* empty */ Lisp_Object user_cmp_function; /* C function to compare two keys. */ - bool (*cmpfn) (struct hash_table_test *t, Lisp_Object, Lisp_Object); + Lisp_Object (*cmpfn) (Lisp_Object, Lisp_Object, struct hash_table_test *t); /* C function to compute hash code. */ - EMACS_UINT (*hashfn) (struct hash_table_test *t, Lisp_Object); + Lisp_Object (*hashfn) (Lisp_Object, struct hash_table_test *t); }; struct Lisp_Hash_Table @@ -3591,13 +3591,13 @@ #define CONS_TO_INTEGER(cons, type, var) \ extern char *extract_data_from_object (Lisp_Object, ptrdiff_t *, ptrdiff_t *); EMACS_UINT hash_string (char const *, ptrdiff_t); EMACS_UINT sxhash (Lisp_Object, int); -EMACS_UINT hashfn_eql (struct hash_table_test *ht, Lisp_Object key); -EMACS_UINT hashfn_equal (struct hash_table_test *ht, Lisp_Object key); +Lisp_Object hashfn_eql (Lisp_Object, struct hash_table_test *); +Lisp_Object hashfn_equal (Lisp_Object, struct hash_table_test *); Lisp_Object make_hash_table (struct hash_table_test, EMACS_INT, float, float, Lisp_Object, bool); -ptrdiff_t hash_lookup (struct Lisp_Hash_Table *, Lisp_Object, EMACS_UINT *); +ptrdiff_t hash_lookup (struct Lisp_Hash_Table *, Lisp_Object, Lisp_Object *); ptrdiff_t hash_put (struct Lisp_Hash_Table *, Lisp_Object, Lisp_Object, - EMACS_UINT); + Lisp_Object); void hash_remove_from_table (struct Lisp_Hash_Table *, Lisp_Object); extern struct hash_table_test const hashtest_eq, hashtest_eql, hashtest_equal; extern void validate_subarray (Lisp_Object, Lisp_Object, Lisp_Object, diff --git a/src/lread.c b/src/lread.c index 3152fcf867..eecb5e141d 100644 --- a/src/lread.c +++ b/src/lread.c @@ -3161,8 +3161,7 @@ read1 (Lisp_Object readcharfun, int *pch, bool first_in_list) Lisp_Object placeholder = Fcons (Qnil, Qnil); struct Lisp_Hash_Table *h = XHASH_TABLE (read_objects_map); - EMACS_UINT hash; - Lisp_Object number = make_fixnum (n); + Lisp_Object number = make_fixnum (n), hash; ptrdiff_t i = hash_lookup (h, number, &hash); if (i >= 0) diff --git a/src/macfont.m b/src/macfont.m index 301951f34a..7170e80140 100644 --- a/src/macfont.m +++ b/src/macfont.m @@ -986,8 +986,7 @@ static void mac_font_get_glyphs_for_variants (CFDataRef, UTF32Char, { struct Lisp_Hash_Table *h; ptrdiff_t i; - EMACS_UINT hash; - Lisp_Object value; + Lisp_Object hash, value; if (!HASH_TABLE_P (macfont_family_cache)) macfont_family_cache = CALLN (Fmake_hash_table, QCtest, Qeq); diff --git a/src/profiler.c b/src/profiler.c index 87be30acc3..e9b6a37d06 100644 --- a/src/profiler.c +++ b/src/profiler.c @@ -36,11 +36,9 @@ saturated_add (EMACS_INT a, EMACS_INT b) typedef struct Lisp_Hash_Table log_t; -static bool cmpfn_profiler ( - struct hash_table_test *, Lisp_Object, Lisp_Object); - -static EMACS_UINT hashfn_profiler ( - struct hash_table_test *, Lisp_Object); +static Lisp_Object cmpfn_profiler (Lisp_Object, Lisp_Object, + struct hash_table_test *); +static Lisp_Object hashfn_profiler (Lisp_Object, struct hash_table_test *); static const struct hash_table_test hashtest_profiler = { @@ -165,7 +163,7 @@ record_backtrace (log_t *log, EMACS_INT count) careful to avoid memory allocation since we're in a signal handler, and we optimize the code to try and avoid computing the hash+lookup twice. See fns.c:Fputhash for reference. */ - EMACS_UINT hash; + Lisp_Object hash; ptrdiff_t j = hash_lookup (log, backtrace, &hash); if (j >= 0) { @@ -529,30 +527,30 @@ DEFUN ("function-equal", Ffunction_equal, Sfunction_equal, 2, 2, 0, return res ? Qt : Qnil; } -static bool -cmpfn_profiler (struct hash_table_test *t, - Lisp_Object bt1, Lisp_Object bt2) +static Lisp_Object +cmpfn_profiler (Lisp_Object bt1, Lisp_Object bt2, struct hash_table_test *t) { if (VECTORP (bt1) && VECTORP (bt2)) { ptrdiff_t l = ASIZE (bt1); if (l != ASIZE (bt2)) - return false; + return Qnil; for (ptrdiff_t i = 0; i < l; i++) if (NILP (Ffunction_equal (AREF (bt1, i), AREF (bt2, i)))) - return false; - return true; + return Qnil; + return Qt; } else - return EQ (bt1, bt2); + return EQ (bt1, bt2) ? Qt : Qnil; } -static EMACS_UINT -hashfn_profiler (struct hash_table_test *ht, Lisp_Object bt) +static Lisp_Object +hashfn_profiler (Lisp_Object bt, struct hash_table_test *ht) { + EMACS_UINT hash; if (VECTORP (bt)) { - EMACS_UINT hash = 0; + hash = 0; ptrdiff_t l = ASIZE (bt); for (ptrdiff_t i = 0; i < l; i++) { @@ -563,10 +561,10 @@ hashfn_profiler (struct hash_table_test *ht, Lisp_Object bt) ? XHASH (XCDR (XCDR (f))) : XHASH (f)); hash = sxhash_combine (hash, hash1); } - return SXHASH_REDUCE (hash); } else - return XHASH (bt); + hash = XHASH (bt); + return make_fixnum (SXHASH_REDUCE (hash)); } static void syms_of_profiler_for_pdumper (void); -- 2.17.1