On 26. 2. 25 15:20, Stefan Sperling wrote:
On Wed, Feb 26, 2025 at 02:54:43PM +0100, Daniel Sahlberg wrote:
Should have researched a bit more before replying. There was already some
code to do exactly this, but it was removed in r1722879:
[[[
stefan2 2016-01-04 15:17:04
Stop using pointer arithmetics to check for proper alignment because
that is not portable.
As a result, platforms that don't allow unaligned data access will
suffer a small additional performance hit.
* subversion/libsvn_subr/eol.c
(svn_eol__find_eol_start): No longer attempt aligned chunky processing
when unaligned access is not supported.
* subversion/libsvn_subr/utf_validate.c
(first_non_fsm_start_char): Same.
]]]
Don't quite understand why but there is probably a reasonable explanation.
It took us a few rounds of testing to keep things working across
all platforms when these optimizations went in.
As far as I recall, there was run-time fallout on the OpenBSD/sparc64
buildbot (which no longer exists; I don't have sparc64 hardware anymore).
Unaligned access will always cause hardware traps on that platform.
The above change was probably related to that.
With x86 platform behaviour changing all the testing we did back then
is being invalidated. The solution with the least amount of effort on
our part would be using the simple char * loop implementation everywhere.
The maintenance effort required to keep the unaligned-access code path
working and safe on all (modern) platforms would likely exceed our
current development capabilities. Though of course, if someone wants
to do that work, I wouldn't be opposed.
Actually, x86 behaviour hasn't changed wrt alignment for "normal" memory
access in decades. What changed was the addition of vector instructions,
which explicitly disallow unaligned access, and compiler support for
them -- in the reported case it's the -mavx that does it, while I
suspect clang just adds something similar by default at high enough
optimization levels.
At the time, IIRC stefan² tested performance on some (then-)modern x86
CPUs and came away with the result that it's faster to allow the CPU to
handle unaligned access ... or it would be. These kinds of optimizations
always smelled slightly off to me, e.g., why not allow compiler
intrinsics to do their magic instead?
For my €.02, I'd just remove this macro and the micro-optimizations that
go with it and, wherever possible, use standard library functions
instead of the handcrafted ones.
Or, since the macro is public -- deprecate it and stop using it in our
code. The workaround for the reported bug would be adding
-DSVN_UNALIGNED_ACCESS_IS_OK=0 to the CFLAGS.
-- Brane
