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

Unable to compile cryptlib #972

Closed
efmer opened this issue Sep 30, 2020 · 4 comments
Closed

Unable to compile cryptlib #972

efmer opened this issue Sep 30, 2020 · 4 comments

Comments

@efmer
Copy link

efmer commented Sep 30, 2020

Windows 10, using the latest Microsoft Visual Studio 2019.

Build cryptlib (cryptlib.vcxproj) fails:

1>cpu.cpp(314): error C3861: '_xgetbv': identifier not found
// Visual Studio 2010 and above, 32 and 64-bit
#if defined(_MSC_VER) && (_MSC_VER >= 1600)

314 >>>>	return _xgetbv(num);

// Visual Studio 2008 and below, 64-bit
#elif defined(_MSC_VER) && defined(_M_X64)

	return XGETBV64(num);

// Visual Studio 2008 and below, 32-bit
#elif defined(_MSC_VER) && defined(_M_IX86)

I changed it to return XGETBV64(num); in the 64 bit build that builds.

@noloader
Copy link
Collaborator

noloader commented Sep 30, 2020

It looks like we are missing the header file <immintrin.h> for Microsoft compilers. And according to this Mozilla bug, it looks like Microsoft's _xgetbv intrinsic is available with VS2010 SP1.

I think we can do one of two things. First, key on VS2010 SP1. I normally try to use the readily available compiler support, like below.

diff --git a/cpu.cpp b/cpu.cpp
index 287d3a9a..50c3170f 100644
--- a/cpu.cpp
+++ b/cpu.cpp
@@ -308,8 +308,8 @@ extern bool CPU_ProbeSSE2();
 // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85684.
 word64 XGetBV(word32 num)
 {
-// Visual Studio 2010 and above, 32 and 64-bit
-#if defined(_MSC_VER) && (_MSC_VER >= 1600)
+// Visual Studio 2010 SP1 and above, 32 and 64-bit
+#if defined(_MSC_VER) && (_MSC_FULL_VER >= 160040219)

        return _xgetbv(num);

@@ -349,7 +349,7 @@ word64 XGetBV(word32 num)
        return (static_cast<word64>(d) << 32) | a;

 // Remainder of GCC and compatibles.
-#else
+#elif

        // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71659 and
        // http://www.agner.org/optimize/vectorclass/read.php?i=65
@@ -360,6 +360,8 @@ word64 XGetBV(word32 num)
                : "=a"(a), "=d"(d) : "c"(num) : "cc"
        );
        return (static_cast<word64>(d) << 32) | a;
+#else
+       # error "Need an xgetbv function"
 #endif
 }

Second, we can avoid _xgetbv intrinsic completely. 32-bit will use the inline ASM; and 64-bit will use XGETBV64 which is ASM in x64dll.asm. The second choice will completely delete the #if defined(_MSC_VER) && (_MSC_FULL_VER >= 160040219) block above.

Do you have a preference?

@noloader
Copy link
Collaborator

This should be cleared with Commit ba286e6780aa and Commit 38e6763249c7.

There were two commits because I forgot to transfer an updated cpu.cpp to my check-in machine after testing.

@wyattoday
Copy link
Contributor

This commit has a bug. Namely, AMR64 target in MSVC compilers can't include that file.

#if !defined(_M_IX86) && !defined(_M_X64)
#error This header is specific to X86 and X64 targets
#endif

The fix, in cpu.cpp:

// For _xgetbv on Microsoft 32-bit and 64-bit platforms
// https://github.com/weidai11/cryptopp/issues/972
#if _MSC_VER >= 1600 && (CRYPTOPP_BOOL_X86 || CRYPTOPP_BOOL_X32 || CRYPTOPP_BOOL_X64)
# include <immintrin.h>
#endif

noloader added a commit that referenced this issue Nov 21, 2020
@noloader
Copy link
Collaborator

Thanks @wyattoday.

This should be cleared (again!) with Commit 7177df3068b6.

EAddario pushed a commit to EAddario/cryptopp that referenced this issue Dec 30, 2020
EAddario pushed a commit to EAddario/cryptopp that referenced this issue Dec 30, 2020
mouse07410 pushed a commit to mouse07410/cryptopp that referenced this issue Feb 26, 2021
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

3 participants