Re: HAVE_FAST_UNALIGNED_ACCESS

["Arsen Arsenović" <arsen@aarsen.me>]

[-- Attachment #1.1: Type: text/plain, Size: 733 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

> I'm still unconvinced, and I said already what will have a chance of
> convincing me: a specific report about a problem this particular code
> causes on a specific existing platform we support in Emacs 29 and with
> a specific compiler.

Similar (but not exactly the same) loops as this one have been shown to
generate incorrect code in this thread.  It's not a large leap for it to
happen to this one, introducing subtle errors for a bit of code that is
completely unnecessary (as demonstrated by it being optional),
especially at higher optimization levels, where the compiler could
easily produce better code than the assumption of a 'mov' would.

Is the following trivial enough for 29?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: [PATCH] Remove aliasing violation in Fstring_lessp --]
[-- Type: text/x-patch, Size: 1226 bytes --]

From 96d75e78358d6c2643bfb7cc65744b8a6178c9d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arsen=20Arsenovi=C4=87?= <arsen@aarsen.me>
Date: Sat, 1 Apr 2023 14:25:12 +0200
Subject: [PATCH] Remove aliasing violation in Fstring_lessp

* src/fns.c (Fstring_lessp) <HAVE_FAST_UNALIGNED_ACCESS>: Remove
strict aliasing violation.
---
 src/fns.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/fns.c b/src/fns.c
index ff364c6..e3e11e2 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -499,10 +499,16 @@ DEFUN ("string-lessp", Fstring_lessp, Sstring_lessp, 2, 2, 0,
 	  /* First compare entire machine words.  */
 	  typedef size_t word_t;
 	  int ws = sizeof (word_t);
-	  const word_t *w1 = (const word_t *) SDATA (string1);
-	  const word_t *w2 = (const word_t *) SDATA (string2);
-	  while (b < nb - ws + 1 && w1[b / ws] == w2[b / ws])
-	    b += ws;
+	  while (b < nb - ws + 1)
+	    {
+	      word_t w1;
+	      word_t w2;
+	      memcpy (&w1, SDATA (string1) + b, sizeof (w1));
+	      memcpy (&w2, SDATA (string2) + b, sizeof (w2));
+	      if (w1 != w2)
+		break;
+	      b += ws;
+	    }
 	}
 
       /* Scan forward to the differing byte.  */
-- 
2.40.0


[-- Attachment #1.3: Type: text/plain, Size: 492 bytes --]


.. or something similar to it, assuming I made an error, which is likely
given the circumstances.  This does pass the testsuite, anyway.  It
should just expand deferences into explicit memcpys.

No actual memcpy calls are produced, and this is at least functional on
a superset of compilers, and I suspect replacing the whole thing with a
naive-looking while (*(w1++) != *(w2++)); loop would be even better (but
I can settle for that being too experimental).
-- 
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 251 bytes --]