Home Original page

[Python-Dev] bpo-28055: Fix unaligned accesses in siphash24(). (GH-6123)

Serhiy Storchaka storchaka at gmail.com
Mon May 14 13:03:22 EDT 2018 
13.05.18 20:42, Christian Heimes пише:
> I was against the approach a good reason. The PR adds additional CPU
> instructions and changes memory access pattern in a  critical path of
> CPython. There is no reason to add additional overhead for the majority
> of users or X86 and X86_64 architectures. The memcpy() call should only
> be used on architectures that do not support unaligned memory access.
> See comment https://bugs.python.org/issue28055#msg276257
> 
> At least for latest GCC, the change seems to be fine. GCC emits the same
> assembly code for X86_64 before and after your change. Did you check the
> output on other CPU architectures as well as clang and MSVC, too?

For the initial implementation of pyhash.c [1] I proposed a patch that 
use memcpy() conditionally to avoid an overhead on Windows:

+#ifdef _MSC_VER
+        block.value = *(const Py_uhash_t*)p;
+#else
+        memcpy(block.bytes, p, SIZEOF_PY_UHASH_T);
+#endif

(and similar code for FNV).

But many developers confirmed that all modern compilers including latest 
versions of MS VS optimize memcpy() with a constant size into a single 
CPU instruction. Seems avoiding to use memcpy() no longer needed.

If using memcpy() adds an overhead on some platforms we can return to 
using a compiler/platform depending code.

[1] https://bugs.python.org/issue19183
More information about the Python-Dev mailing list