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

Incorrect using __cpuidex intrinsic under MSVC #45

Closed
alukutkin opened this issue Nov 7, 2016 · 3 comments
Closed

Incorrect using __cpuidex intrinsic under MSVC #45

alukutkin opened this issue Nov 7, 2016 · 3 comments

Comments

@alukutkin
Copy link

Hello.
In function simdpp::detail::get_cpuid i found one little error with big effects.

Original code:

...
#elif _MSC_VER
    uint32_t regs[4];
    __cpuidex((int*) regs, subleaf, level);
    *eax = regs[0];
    *ebx = regs[1];
    *ecx = regs[2];
    *edx = regs[3];
#else
...

But if you'll see MSDN (https://msdn.microsoft.com/ru-ru/library/hskdteyh.aspx) you can notice what subleaf and level params followed in the wrong order. If you change that order all works as intended.

You can use some simple test:

#include <iostream>
#include <simdpp/simd.h>
#include <simdpp/dispatch/get_arch_raw_cpuid.h>

#define SIMDPP_USER_ARCH_INFO ::simdpp::get_arch_raw_cpuid()

namespace SIMDPP_ARCH_NAMESPACE {
std::string archToString(simdpp::Arch arch)
{
    std::string ret = "none";

    if ((arch & simdpp::Arch::X86_SSE2) == simdpp::Arch::X86_SSE2)
        ret += " sse2";
    if ((arch & simdpp::Arch::X86_SSE3) == simdpp::Arch::X86_SSE3)
        ret += " sse3";
    if ((arch & simdpp::Arch::X86_SSSE3) == simdpp::Arch::X86_SSSE3)
        ret += " ssse3";
    if ((arch & simdpp::Arch::X86_SSE4_1) == simdpp::Arch::X86_SSE4_1)
        ret += " sse4.1";
    if ((arch & simdpp::Arch::X86_FMA3) == simdpp::Arch::X86_FMA3)
        ret += " fma3";
    if ((arch & simdpp::Arch::X86_FMA4) == simdpp::Arch::X86_FMA4)
        ret += " fma4";
    if ((arch & simdpp::Arch::X86_XOP) == simdpp::Arch::X86_XOP)
        ret += " xop";
    if ((arch & simdpp::Arch::X86_AVX) == simdpp::Arch::X86_AVX)
        ret += " avx";
    if ((arch & simdpp::Arch::X86_AVX2) == simdpp::Arch::X86_AVX2)
        ret += " avx2";
    if ((arch & simdpp::Arch::X86_AVX512F) == simdpp::Arch::X86_AVX512F)
        ret += " avx512f";

    return ret;
}

void printArch()
{
    std::cout << "cpu arch: " << archToString(SIMDPP_USER_ARCH_INFO).c_str();
    std::cout << std::endl;

    std::cout << "compile arch: " << archToString(simdpp::this_compile_arch()).c_str();
    std::cout << std::endl;
}

} // namespace SIMDPP_ARCH_NAMESPACE

SIMDPP_MAKE_DISPATCHER_VOID0(printArch)

I'm tested it on Microsoft C++ Build Tools (based on MSVC 2015 SP3) with my i5-4460 CPU.

Output before fix:

cpu arch: none fma3
compile arch: none

Output after fix:

cpu arch: none sse2 sse3 ssse3 sse4.1 fma3 avx avx2
compile arch: none sse2 sse3 ssse3 sse4.1 fma3 avx

Sorry me, but I can't create pull request at that time :(

@alukutkin
Copy link
Author

I added pull requests to fix that problem in master and cxx98 branches

@p12tic
Copy link
Owner

p12tic commented Nov 9, 2016

Thanks a lot. I'll merge them when I find out why the test farm hasn't caught the bug and update the tests.

@p12tic
Copy link
Owner

p12tic commented Jul 28, 2017

Fixes have been merged to both master and cxx98 branches. Thanks!

@p12tic p12tic closed this as completed Jul 28, 2017
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