Remove UNSIGNED_CMP

I added this macro in 2011 to fix some signedness comparison bugs.
However, it’s a weird macro and the bugs can be fixed in a
more-straightforward way.  This helps performance slightly (0.5%) on my
platform (Fedora 31 x86-64, GCC 9.3.1 with -O2).
* src/casefiddle.c (do_casify_natnum): Use simple comparison
instead of UNSIGNED_CMP.
* src/character.h (CHAR_VALID_P, SINGLE_BYTE_CHAR_P, CHAR_STRING):
* src/composite.h (COMPOSITION_ENCODE_RULE_VALID):
* src/lisp.h (ASCII_CHAR_P):
Now an inline function, and uses simple comparison instead of
UNSIGNED_CMP.
* src/dispextern.h (FACE_FROM_ID, FACE_FROM_ID_OR_NULL)
(IMAGE_FROM_ID, IMAGE_OPT_FROM_ID): Move these to ...
* src/frame.h (FACE_FROM_ID, FACE_FROM_ID_OR_NULL)
(IMAGE_FROM_ID, IMAGE_OPT_FROM_ID): ... here, and make them
inline functions that no longer use UNSIGNED_CMP.
* src/keyboard.c (read_char): UNSIGNED_CMP is not needed here
since XFIXNAT always returns a nonnegative value.
* src/lisp.h (UNSIGNED_CMP): Remove; no longer used.

7 files changed, 97 insertions, 67 deletions

diff --git a/src/casefiddle.c b/src/casefiddle.c
index 1945aa15e71..5018b7bb1cd 100644
--- a/src/casefiddle.c
+++ b/src/casefiddle.c
@@ -229,7 +229,7 @@ do_casify_natnum (struct casing_context *ctx, Lisp_Object obj)
 
   /* If the character has higher bits set above the flags, return it unchanged.
      It is not a real character.  */
-  if (UNSIGNED_CMP (ch, >, flagbits))
+  if (! (0 <= ch && ch <= flagbits))
     return obj;
 
   int flags = ch & flagbits;
diff --git a/src/character.h b/src/character.h
index 3642a540448..7639b018cb0 100644
--- a/src/character.h
+++ b/src/character.h
@@ -80,6 +80,8 @@ enum
   OBJECT_REPLACEMENT_CHARACTER = 0xFFFC,
 };
 
+extern int char_string (unsigned, unsigned char *);
+
 /* UTF-8 encodings.  Use \x escapes, so they are portable to pre-C11
    compilers and can be concatenated with ordinary string literals.  */
 #define uLSQM "\xE2\x80\x98" /* U+2018 LEFT SINGLE QUOTATION MARK */
@@ -126,7 +128,11 @@ enum
 #define CHARACTERP(x) (FIXNATP (x) && XFIXNAT (x) <= MAX_CHAR)
 
 /* Nonzero iff C is valid as a character code.  */
-#define CHAR_VALID_P(c) UNSIGNED_CMP (c, <=, MAX_CHAR)
+INLINE bool
+CHAR_VALID_P (intmax_t c)
+{
+  return 0 <= c && c <= MAX_CHAR;
+}
 
 /* Check if Lisp object X is a character or not.  */
 #define CHECK_CHARACTER(x) \
@@ -145,7 +151,11 @@ enum
   } while (false)
 
 /* Nonzero iff C is a character of code less than 0x100.  */
-#define SINGLE_BYTE_CHAR_P(c) UNSIGNED_CMP (c, <, 0x100)
+INLINE bool
+SINGLE_BYTE_CHAR_P (intmax_t c)
+{
+  return 0 <= c && c < 0x100;
+}
 
 /* Nonzero if character C has a printable glyph.  */
 #define CHAR_PRINTABLE_P(c)     \
@@ -176,20 +186,32 @@ enum
    allocate at least MAX_MULTIBYTE_LENGTH bytes area at P in advance.
    Returns the length of the multibyte form.  */
 
-#define CHAR_STRING(c, p)                       \
-  (UNSIGNED_CMP (c, <=, MAX_1_BYTE_CHAR)        \
-   ? ((p)[0] = (c),                             \
-      1)                                        \
-   : UNSIGNED_CMP (c, <=, MAX_2_BYTE_CHAR)      \
-   ? ((p)[0] = (0xC0 | ((c) >> 6)),             \
-      (p)[1] = (0x80 | ((c) & 0x3F)),           \
-      2)                                        \
-   : UNSIGNED_CMP (c, <=, MAX_3_BYTE_CHAR)      \
-   ? ((p)[0] = (0xE0 | ((c) >> 12)),            \
-      (p)[1] = (0x80 | (((c) >> 6) & 0x3F)),    \
-      (p)[2] = (0x80 | ((c) & 0x3F)),           \
-      3)                                        \
-   : verify_expr (sizeof (c) <= sizeof (unsigned), char_string (c, p)))
+INLINE int
+CHAR_STRING (int c, unsigned char *p)
+{
+  eassume (0 <= c);
+  if (c <= MAX_1_BYTE_CHAR)
+    {
+      p[0] = c;
+      return 1;
+    }
+  if (c <= MAX_2_BYTE_CHAR)
+    {
+      p[0] = 0xC0 | (c >> 6);
+      p[1] = 0x80 | (c & 0x3F);
+      return 2;
+    }
+  if (c <= MAX_3_BYTE_CHAR)
+    {
+      p[0] = 0xE0 | (c >> 12);
+      p[1] = 0x80 | ((c >> 6) & 0x3F);
+      p[2] = 0x80 | (c & 0x3F);
+      return 3;
+    }
+  int len = char_string (c, p);
+  eassume (0 < len && len <= MAX_MULTIBYTE_LENGTH);
+  return len;
+}
 
 /* Store multibyte form of byte B in P.  The caller should allocate at
    least MAX_MULTIBYTE_LENGTH bytes area at P in advance.  Returns the
@@ -657,7 +679,6 @@ typedef enum {
 } unicode_category_t;
 
 extern EMACS_INT char_resolve_modifier_mask (EMACS_INT) ATTRIBUTE_CONST;
-extern int char_string (unsigned, unsigned char *);
 extern int string_char (const unsigned char *,
                         const unsigned char **, int *);
 
diff --git a/src/composite.h b/src/composite.h
index 62c4de40e3b..239f1e531ef 100644
--- a/src/composite.h
+++ b/src/composite.h
@@ -125,10 +125,13 @@ composition_registered_p (Lisp_Object prop)
     COMPOSITION_DECODE_REFS (rule_code, gref, nref);                    \
   } while (false)
 
-/* Nonzero if the global reference point GREF and new reference point NREF are
+/* True if the global reference point GREF and new reference point NREF are
    valid.  */
-#define COMPOSITION_ENCODE_RULE_VALID(gref, nref)       \
-  (UNSIGNED_CMP (gref, <, 12) && UNSIGNED_CMP (nref, <, 12))
+INLINE bool
+COMPOSITION_ENCODE_RULE_VALID (int gref, int nref)
+{
+  return 0 <= gref && gref < 12 && 0 <= nref && nref < 12;
+}
 
 /* Return encoded composition rule for the pair of global reference
    point GREF and new reference point NREF.  Arguments must be valid.  */
diff --git a/src/dispextern.h b/src/dispextern.h
index 555946f84cb..ae994d7f9bc 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1855,20 +1855,6 @@ struct face_cache
   bool_bf menu_face_changed_p : 1;
 };
 
-/* Return a non-null pointer to the cached face with ID on frame F.  */
-
-#define FACE_FROM_ID(F, ID)                                     \
-  (eassert (UNSIGNED_CMP (ID, <, FRAME_FACE_CACHE (F)->used)),  \
-   FRAME_FACE_CACHE (F)->faces_by_id[ID])
-
-/* Return a pointer to the face with ID on frame F, or null if such a
-   face doesn't exist.  */
-
-#define FACE_FROM_ID_OR_NULL(F, ID)                     \
-  (UNSIGNED_CMP (ID, <, FRAME_FACE_CACHE (F)->used)     \
-   ? FRAME_FACE_CACHE (F)->faces_by_id[ID]              \
-   : NULL)
-
 #define FACE_EXTENSIBLE_P(F)                    \
   (!NILP (F->lface[LFACE_EXTEND_INDEX]))
 
@@ -3163,21 +3149,6 @@ struct image_cache
   ptrdiff_t refcount;
 };
 
-
-/* A non-null pointer to the image with id ID on frame F.  */
-
-#define IMAGE_FROM_ID(F, ID)                                    \
-  (eassert (UNSIGNED_CMP (ID, <, FRAME_IMAGE_CACHE (F)->used)), \
-   FRAME_IMAGE_CACHE (F)->images[ID])
-
-/* Value is a pointer to the image with id ID on frame F, or null if
-   no image with that id exists.  */
-
-#define IMAGE_OPT_FROM_ID(F, ID)                                \
-  (UNSIGNED_CMP (ID, <, FRAME_IMAGE_CACHE (F)->used)            \
-   ? FRAME_IMAGE_CACHE (F)->images[ID]                          \
-   : NULL)
-
 /* Size of bucket vector of image caches.  Should be prime.  */
 
 #define IMAGE_CACHE_BUCKETS_SIZE 1001
diff --git a/src/frame.h b/src/frame.h
index 641bb430d0e..476bac67faf 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1450,6 +1450,49 @@ FRAME_BOTTOM_DIVIDER_WIDTH (struct frame *f)
 {
   return frame_dimension (f->bottom_divider_width);
 }
+
+/* Return a non-null pointer to the cached face with ID on frame F.  */
+
+INLINE struct face *
+FACE_FROM_ID (struct frame *f, int id)
+{
+  eassert (0 <= id && id < FRAME_FACE_CACHE (f)->used);
+  return FRAME_FACE_CACHE (f)->faces_by_id[id];
+}
+
+/* Return a pointer to the face with ID on frame F, or null if such a
+   face doesn't exist.  */
+
+INLINE struct face *
+FACE_FROM_ID_OR_NULL (struct frame *f, int id)
+{
+  int used = FRAME_FACE_CACHE (f)->used;
+  eassume (0 <= used);
+  return 0 <= id && id < used ? FRAME_FACE_CACHE (f)->faces_by_id[id] : NULL;
+}
+
+#ifdef HAVE_WINDOW_SYSTEM
+
+/* A non-null pointer to the image with id ID on frame F.  */
+
+INLINE struct image *
+IMAGE_FROM_ID (struct frame *f, int id)
+{
+  eassert (0 <= id && id < FRAME_IMAGE_CACHE (f)->used);
+  return FRAME_IMAGE_CACHE (f)->images[id];
+}
+
+/* Value is a pointer to the image with id ID on frame F, or null if
+   no image with that id exists.  */
+
+INLINE struct image *
+IMAGE_OPT_FROM_ID (struct frame *f, int id)
+{
+  int used = FRAME_IMAGE_CACHE (f)->used;
+  eassume (0 <= used);
+  return 0 <= id && id < used ? FRAME_IMAGE_CACHE (f)->images[id] : NULL;
+}
+#endif
 
 /***********************************************************************
             Conversion between canonical units and pixels
diff --git a/src/keyboard.c b/src/keyboard.c
index 9ce168c6dd0..b4e62c3bb49 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2927,13 +2927,11 @@ read_char (int commandflag, Lisp_Object map,
         goto exit;
 
       if ((STRINGP (KVAR (current_kboard, Vkeyboard_translate_table))
-           && UNSIGNED_CMP (XFIXNAT (c), <,
-                            SCHARS (KVAR (current_kboard,
-                                          Vkeyboard_translate_table))))
+           && XFIXNAT (c) < SCHARS (KVAR (current_kboard,
+                                          Vkeyboard_translate_table)))
           || (VECTORP (KVAR (current_kboard, Vkeyboard_translate_table))
-              && UNSIGNED_CMP (XFIXNAT (c), <,
-                               ASIZE (KVAR (current_kboard,
-                                            Vkeyboard_translate_table))))
+              && XFIXNAT (c) < ASIZE (KVAR (current_kboard,
+                                            Vkeyboard_translate_table)))
           || (CHAR_TABLE_P (KVAR (current_kboard, Vkeyboard_translate_table))
               && CHARACTERP (c)))
         {
diff --git a/src/lisp.h b/src/lisp.h
index c3efabaf528..706ca6b9a82 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1923,18 +1923,12 @@ memclear (void *p, ptrdiff_t nbytes)
   (offsetof (type, lastlispfield) + word_size < header_size             \
    ? 0 : (offsetof (type, lastlispfield) + word_size - header_size) / word_size)
 
-/* Compute A OP B, using the unsigned comparison operator OP.  A and B
-   should be integer expressions.  This is not the same as
-   mathematical comparison; for example, UNSIGNED_CMP (0, <, -1)
-   returns true.  For efficiency, prefer plain unsigned comparison if A
-   and B's sizes both fit (after integer promotion).  */
-#define UNSIGNED_CMP(a, op, b)                                          \
-  (max (sizeof ((a) + 0), sizeof ((b) + 0)) <= sizeof (unsigned)        \
-   ? ((a) + (unsigned) 0) op ((b) + (unsigned) 0)                       \
-   : ((a) + (uintmax_t) 0) op ((b) + (uintmax_t) 0))
-
 /* True iff C is an ASCII character.  */
-#define ASCII_CHAR_P(c) UNSIGNED_CMP (c, <, 0x80)
+INLINE bool
+ASCII_CHAR_P (intmax_t c)
+{
+  return 0 <= c && c < 0x80;
+}
 
 /* A char-table is a kind of vectorlike, with contents are like a
    vector but with a few other slots.  For some purposes, it makes