From ffaa292006b8fdc68e4e9de16fea066d51cc2735 Mon Sep 17 00:00:00 2001 From: PavelP Date: Wed, 22 Apr 2020 00:42:53 +0600 Subject: [PATCH] Master vs2019 x86 compile fixes (#743) * Added bitexact implementations of _BitScanForward64 and _BitScanReverse64 for VS2019 32-bit builds * Added bitexact implementations of _umul128 for VS2019 x86, arm, arm64 builds * Implement mul_overflow for VS2019 arm64 builds + implement mul_overflow using __umulh (msvc/clang results: https://godbolt.org/z/smRwA7) * Added Win32 for VS2019 to .appveyor.yml * Update amalgamated headers (fix x86 builds with VS2019) --- .appveyor.yml | 7 ++++- singleheader/simdjson.cpp | 47 ++++++++++++++++++++++++++-------- src/arm64/bitmanipulation.h | 10 ++------ src/fallback/bitmanipulation.h | 17 ++++++++++++ src/jsoncharutils.h | 18 ++++++++++++- 5 files changed, 79 insertions(+), 20 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index cdd1242f6..7ed59bb6a 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -12,14 +12,19 @@ environment: matrix: - SIMDJSON_BUILD_STATIC: ON SIMDJSON_ENABLE_THREADS: OFF + SIMDJSON_PLATFORM: x64 + - SIMDJSON_BUILD_STATIC: ON + SIMDJSON_ENABLE_THREADS: OFF + SIMDJSON_PLATFORM: Win32 - SIMDJSON_BUILD_STATIC: OFF SIMDJSON_ENABLE_THREADS: ON + SIMDJSON_PLATFORM: x64 build_script: - set - mkdir build - cd build - - cmake -DSIMDJSON_BUILD_STATIC=%SIMDJSON_BUILD_STATIC% -DSIMDJSON_ENABLE_THREADS=%SIMDJSON_ENABLE_THREADS% -DCMAKE_BUILD_TYPE=%Configuration% -DCMAKE_GENERATOR_PLATFORM=x64 .. + - cmake -DSIMDJSON_BUILD_STATIC=%SIMDJSON_BUILD_STATIC% -DSIMDJSON_ENABLE_THREADS=%SIMDJSON_ENABLE_THREADS% -DCMAKE_BUILD_TYPE=%Configuration% -DCMAKE_GENERATOR_PLATFORM=%SIMDJSON_PLATFORM% .. - cmake -LH .. - cmake --build . --config %Configuration% --verbose diff --git a/singleheader/simdjson.cpp b/singleheader/simdjson.cpp index 2922a8ce9..8edef0ed4 100644 --- a/singleheader/simdjson.cpp +++ b/singleheader/simdjson.cpp @@ -697,16 +697,10 @@ really_inline bool add_overflow(uint64_t value1, uint64_t value2, uint64_t *resu #endif } -#ifdef _MSC_VER -#pragma intrinsic(_umul128) // todo: this might fail under visual studio for ARM -#endif - really_inline bool mul_overflow(uint64_t value1, uint64_t value2, uint64_t *result) { #ifdef _MSC_VER - // todo: this might fail under visual studio for ARM - uint64_t high; - *result = _umul128(value1, value2, &high); - return high; + *result = value1 * value2; + return !!__umulh(value1, value2); #else return __builtin_umulll_overflow(value1, value2, (unsigned long long *)result); #endif @@ -5759,7 +5753,7 @@ inline size_t codepoint_to_utf8(uint32_t cp, uint8_t *c) { // The following code is used in number parsing. It is not // properly "char utils" stuff, but we move it here so that // it does not get copied multiple times in the binaries (once -// per instructin set). +// per instruction set). /// @@ -5771,10 +5765,26 @@ struct value128 { uint64_t high; }; +#if defined(_MSC_VER) && !defined(_M_X64) // _umul128 for x86, arm, arm64 +#if defined(_M_ARM) +static inline uint64_t __emulu(uint32_t x, uint32_t y) { + return x * (uint64_t)y; +} +#endif +static inline uint64_t _umul128(uint64_t ab, uint64_t cd, uint64_t *hi) { + uint64_t ad = __emulu((uint32_t)(ab >> 32), (uint32_t)cd); + uint64_t bd = __emulu((uint32_t)ab, (uint32_t)cd); + uint64_t adbc = ad + __emulu((uint32_t)ab, (uint32_t)(cd >> 32)); + uint64_t adbc_carry = !!(adbc < ad); + uint64_t lo = bd + (adbc << 32); + *hi = __emulu((uint32_t)(ab >> 32), (uint32_t)(cd >> 32)) + (adbc >> 32) + (adbc_carry << 32) + !!(lo < bd); + return lo; +} +#endif + really_inline value128 full_multiplication(uint64_t value1, uint64_t value2) { value128 answer; #ifdef _MSC_VER - // todo: this might fail under visual studio for ARM answer.low = _umul128(value1, value2, &answer.high); #else __uint128_t r = ((__uint128_t)value1) * value2; @@ -8559,6 +8569,23 @@ WARN_UNUSED really_inline uint8_t *parse_string(const uint8_t *src, uint8_t *dst namespace simdjson { namespace fallback { +#if defined(_MSC_VER) && !defined(_M_ARM64) && !defined(_M_X64) +static inline unsigned char _BitScanForward64(unsigned long* ret, uint64_t x) { + unsigned long x0 = (unsigned long)x, top, bottom; + _BitScanForward(&top, (unsigned long)(x >> 32)); + _BitScanForward(&bottom, x0); + *ret = x0 ? bottom : 32 + top; + return x != 0; +} +static unsigned char _BitScanReverse64(unsigned long* ret, uint64_t x) { + unsigned long x1 = (unsigned long)(x >> 32), top, bottom; + _BitScanReverse(&top, x1); + _BitScanReverse(&bottom, (unsigned long)x); + *ret = x1 ? top + 32 : bottom; + return x != 0; +} +#endif + // We sometimes call trailing_zero on inputs that are zero, // but the algorithms do not end up using the returned value. // Sadly, sanitizers are not smart enough to figure it out. diff --git a/src/arm64/bitmanipulation.h b/src/arm64/bitmanipulation.h index 6229d84e5..b3d609fee 100644 --- a/src/arm64/bitmanipulation.h +++ b/src/arm64/bitmanipulation.h @@ -59,16 +59,10 @@ really_inline bool add_overflow(uint64_t value1, uint64_t value2, uint64_t *resu #endif } -#ifdef _MSC_VER -#pragma intrinsic(_umul128) // todo: this might fail under visual studio for ARM -#endif - really_inline bool mul_overflow(uint64_t value1, uint64_t value2, uint64_t *result) { #ifdef _MSC_VER - // todo: this might fail under visual studio for ARM - uint64_t high; - *result = _umul128(value1, value2, &high); - return high; + *result = value1 * value2; + return !!__umulh(value1, value2); #else return __builtin_umulll_overflow(value1, value2, (unsigned long long *)result); #endif diff --git a/src/fallback/bitmanipulation.h b/src/fallback/bitmanipulation.h index 96a557fa4..3574b651c 100644 --- a/src/fallback/bitmanipulation.h +++ b/src/fallback/bitmanipulation.h @@ -7,6 +7,23 @@ namespace simdjson { namespace fallback { +#if defined(_MSC_VER) && !defined(_M_ARM64) && !defined(_M_X64) +static inline unsigned char _BitScanForward64(unsigned long* ret, uint64_t x) { + unsigned long x0 = (unsigned long)x, top, bottom; + _BitScanForward(&top, (unsigned long)(x >> 32)); + _BitScanForward(&bottom, x0); + *ret = x0 ? bottom : 32 + top; + return x != 0; +} +static unsigned char _BitScanReverse64(unsigned long* ret, uint64_t x) { + unsigned long x1 = (unsigned long)(x >> 32), top, bottom; + _BitScanReverse(&top, x1); + _BitScanReverse(&bottom, (unsigned long)x); + *ret = x1 ? top + 32 : bottom; + return x != 0; +} +#endif + // We sometimes call trailing_zero on inputs that are zero, // but the algorithms do not end up using the returned value. // Sadly, sanitizers are not smart enough to figure it out. diff --git a/src/jsoncharutils.h b/src/jsoncharutils.h index 39d7be9ea..01dd35ec2 100644 --- a/src/jsoncharutils.h +++ b/src/jsoncharutils.h @@ -324,10 +324,26 @@ struct value128 { uint64_t high; }; +#if defined(_MSC_VER) && !defined(_M_X64) // _umul128 for x86, arm, arm64 +#if defined(_M_ARM) +static inline uint64_t __emulu(uint32_t x, uint32_t y) { + return x * (uint64_t)y; +} +#endif +static inline uint64_t _umul128(uint64_t ab, uint64_t cd, uint64_t *hi) { + uint64_t ad = __emulu((uint32_t)(ab >> 32), (uint32_t)cd); + uint64_t bd = __emulu((uint32_t)ab, (uint32_t)cd); + uint64_t adbc = ad + __emulu((uint32_t)ab, (uint32_t)(cd >> 32)); + uint64_t adbc_carry = !!(adbc < ad); + uint64_t lo = bd + (adbc << 32); + *hi = __emulu((uint32_t)(ab >> 32), (uint32_t)(cd >> 32)) + (adbc >> 32) + (adbc_carry << 32) + !!(lo < bd); + return lo; +} +#endif + really_inline value128 full_multiplication(uint64_t value1, uint64_t value2) { value128 answer; #ifdef _MSC_VER - // todo: this might fail under visual studio for ARM answer.low = _umul128(value1, value2, &answer.high); #else __uint128_t r = ((__uint128_t)value1) * value2;