Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_umul128 is not a valid intrinsic for x86 Windows. #3

Closed
Convery opened this issue Apr 2, 2019 · 3 comments
Closed

_umul128 is not a valid intrinsic for x86 Windows. #3

Convery opened this issue Apr 2, 2019 · 3 comments

Comments

@Convery
Copy link
Author

Convery commented Apr 2, 2019

Example fix for line 470-483 as stolen from SO:

#ifdef PHMAP_HAVE_INTRINSIC_INT128
    inline uint64_t umul128(uint64_t a, uint64_t b, uint64_t* high)
    {
        auto result = static_cast<unsigned __int128>(a) * static_cast<unsigned __int128>(b);
        *high = static_cast<uint64_t>(result >> 64);
        return static_cast<uint64_t>(result);
    }
#elif (defined(_MSC_VER))
    #if defined(_WIN64)
    #pragma intrinsic(_umul128)
    inline uint64_t umul128(uint64_t a, uint64_t b, uint64_t* high)
    {
        return _umul128(a, b, high);
    }
    #else
    #pragma intrinsic(__emulu)
    inline uint64_t umul128(uint64_t multiplier, uint64_t multiplicand, uint64_t *product_hi)
    {
        uint64_t a = multiplier >> 32;
        uint64_t b = (uint32_t)multiplier; // & 0xFFFFFFFF;
        uint64_t c = multiplicand >> 32;
        uint64_t d = (uint32_t)multiplicand; // & 0xFFFFFFFF;

        uint64_t ad = __emulu(a, d);
        uint64_t bd = __emulu(b, d);

        uint64_t adbc = ad + __emulu(b, c);
        uint64_t adbc_carry = (adbc < ad); // ? 1 : 0;
        // MSVC gets confused by the ternary and makes worse code than using a boolean in an integer context for 1 : 0

        // multiplier * multiplicand = product_hi * 2^64 + product_lo
        uint64_t product_lo = bd + (adbc << 32);
        uint64_t product_lo_carry = (product_lo < bd); // ? 1 : 0;
        *product_hi = __emulu(a, c) + (adbc >> 32) + (adbc_carry << 32) + product_lo_carry;

        return product_lo;
    }
    #endif
#endif

@greg7mdp
Copy link
Owner

greg7mdp commented Apr 2, 2019

Thanks for the report and suggesting a solution. Not closing the issue yet because there are a couple compilation warnings I need to take care of.

@greg7mdp
Copy link
Owner

greg7mdp commented Apr 6, 2019

I updated the code and checked on x86... works fine now. Thanks again for the bug report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants