Improve MSVC codegen

dotnet · Apr 15, 2024 · af63c5c · af63c5c
1 parent 5083f40
commit af63c5c
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 13 deletions.
diff --git a/src/native/containers/dn-simdhash-arch.h b/src/native/containers/dn-simdhash-arch.h
@@ -192,15 +192,19 @@ build_search_vector (uint8_t needle)
 	return result;
 }
 
-// returns an index in range 0-14 on match, 15-32 if no match
+// returns an index in range 0-13 on match, 14-32 if no match
 static DN_FORCEINLINE(uint32_t)
-find_first_matching_suffix (
-	dn_simdhash_search_vector needle, dn_simdhash_suffixes haystack,
-	uint8_t haystack_values[DN_SIMDHASH_VECTOR_WIDTH], uint32_t count
+find_first_matching_suffix_internal (
+	__m128i needle, __m128i haystack,
+	uint32_t count
 ) {
-	return ctz(_mm_movemask_epi8(_mm_cmpeq_epi8(needle.m128, haystack.m128)));
+	return ctz(_mm_movemask_epi8(_mm_cmpeq_epi8(needle, haystack)));
 }
 
+// use a macro to discard haystack_values, otherwise MSVC's codegen is worse
+#define find_first_matching_suffix(needle, haystack, haystack_values, count) \
+	find_first_matching_suffix_internal(needle.m128, haystack.m128, count)
+
 #else // unknown compiler and/or unknown non-simd arch
 
 #define DN_SIMDHASH_USE_SCALAR_FALLBACK 1

diff --git a/src/native/containers/dn-simdhash-ght-compatible.h b/src/native/containers/dn-simdhash-ght-compatible.h
@@ -18,9 +18,9 @@ dn_simdhash_ght_new_full (
 // compatible with g_hash_table_insert_replace
 void
 dn_simdhash_ght_insert_replace (
-    dn_simdhash_ght_t *hash,
-    void * key, void * value,
-    int32_t overwrite_key
+	dn_simdhash_ght_t *hash,
+	void * key, void * value,
+	int32_t overwrite_key
 );
 
 // compatibility shims for the g_hash_table_ versions in glib.h

diff --git a/src/native/containers/dn-simdhash-specialization.h b/src/native/containers/dn-simdhash-specialization.h
@@ -134,7 +134,11 @@ address_of_value (dn_simdhash_buffers_t buffers, uint32_t value_slot_index)
 static DN_FORCEINLINE(int)
 DN_SIMDHASH_SCAN_BUCKET_INTERNAL (DN_SIMDHASH_T_PTR hash, bucket_t *restrict bucket, DN_SIMDHASH_KEY_T needle, dn_simdhash_search_vector search_vector)
 {
-#ifndef DN_SIMDHASH_USE_SCALAR_FALLBACK
+#ifdef _MSC_VER
+	// MSVC won't do efficient lane extractions if we eager load the vector,
+	//  so just operate through the pointer instead.
+	#define bucket_suffixes (bucket->suffixes)
+#elif !defined(DN_SIMDHASH_USE_SCALAR_FALLBACK)
 	// Perform an eager load of the vector if SIMD is in use, even though we do
 	//  byte loads to extract lanes on non-wasm platforms. It's faster on x64 for
 	//  a reason I can't identify, and it significantly improves wasm codegen
@@ -171,15 +175,15 @@ DN_SIMDHASH_SCAN_BUCKET_INTERNAL (DN_SIMDHASH_T_PTR hash, bucket_t *restrict buc
 // Helper macros so that we can optimize and change scan logic more easily
 #define BEGIN_SCAN_BUCKETS(initial_index, bucket_index, bucket_address) \
 	{ \
-		uint32_t bucket_index = initial_index; \
+		uint32_t bucket_index = initial_index, scan_buckets_length = buffers.buckets_length; \
 		bucket_t *restrict bucket_address = address_of_bucket(buffers, bucket_index); \
 		do {
 
 #define END_SCAN_BUCKETS(initial_index, bucket_index, bucket_address) \
 			bucket_index++; \
 			bucket_address++; \
 			/* Wrap around if we hit the last bucket. */ \
-			if (bucket_index >= buffers.buckets_length) { \
+			if (bucket_index >= scan_buckets_length) { \
 				bucket_index = 0; \
 				bucket_address = address_of_bucket(buffers, 0); \
 			} \

diff --git a/src/native/containers/simdhash-benchmark/run-benchmark.ps1 b/src/native/containers/simdhash-benchmark/run-benchmark.ps1
@@ -1,2 +1,2 @@
-cl /O2 /std:c17 ./*.c ../dn-simdhash-u32-ptr.c ../dn-simdhash.c ../dn-vector.c ../dn-simdhash-string-ptr.c /DNO_CONFIG_H /DSIZEOF_VOID_P=8 /Fe:all-measurements.exe
-./all-measurements.exe
+cl /GS- /O2 /std:c17 ./*.c ../dn-simdhash-u32-ptr.c ../dn-simdhash.c ../dn-vector.c ../dn-simdhash-string-ptr.c /DNO_CONFIG_H /DSIZEOF_VOID_P=8 /Fe:all-measurements.exe
+./all-measurements.exe