From b9e163fa67ea27fffd3d2294f4f1e19b57814aeb Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 29 May 2020 15:23:51 -0700 Subject: util: Avoid strict aliasing bugs in xxhash. XXH32 is doing access through u32 *, and with strict aliasing the compiler gets to assume that those are independent of the u16 writes we did in fd6_texture_key setup, and based on various tweaks to the code, would result in bad hashes computed after inlining. The failure was: ../src/util/hash_table.c:326:_mesa_hash_table_search_pre_hashed: Assertion `ht->key_hash_function == ((void *)0) || hash == ht->key_hash_function(key)' failed.) By setting these two flags, we always take the unaligned, memcpy-the-32-bit-data path. I believe this should be same perf on x86 (which will happily unaligned load 32 bits in the end), while it will be slower on arm (where you have to a special unaligned load operation iirc). This should still be far faster than our old hash. Fixes: edd62619a1c4 ("freedreno: replace fnv1a hash function with xxhash") Acked-by: Rob Clark Part-of: --- src/util/xxhash.h | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src') diff --git a/src/util/xxhash.h b/src/util/xxhash.h index c0c8f44b600..f7a4b40578e 100644 --- a/src/util/xxhash.h +++ b/src/util/xxhash.h @@ -69,6 +69,15 @@ XXH64 13.8 GB/s 1.9 GB/s XXH32 6.8 GB/s 6.0 GB/s */ +/* Mesa leaves strict aliasing on in the compiler, and this code likes to + * dereference the passed in data as u32*, which means that the compiler is + * free to move the u32 read before the write of the struct members being + * hashed, and in practice it did in freedreno. Forcing these two things + * prevents it. + */ +#define XXH_FORCE_ALIGN_CHECK 0 +#define XXH_FORCE_MEMORY_ACCESS 0 + #if defined (__cplusplus) extern "C" { #endif -- cgit v1.2.3