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

XXH3 and Windows ARM builds #220

Closed
carstenskyboxlabs opened this issue Jun 27, 2019 · 6 comments · Fixed by #224
Closed

XXH3 and Windows ARM builds #220

carstenskyboxlabs opened this issue Jun 27, 2019 · 6 comments · Fixed by #224
Labels

Comments

@carstenskyboxlabs
Copy link

I have a problem with using xxh3.h on Widows while building for an ARM architecture.
The problem is that __emulu is an x64/x86 only windows internal macro (MS Docs).

/* U64 XXH_mult32to64(U32 a, U64 b) { return (U64)a * (U64)b; } */
#ifdef _MSC_VER
#   include <intrin.h>
    /* MSVC doesn't do a good job with the mull detection. */
#   define XXH_mult32to64 __emulu
#else
#   define XXH_mult32to64(x, y) ((U64)((x) & 0xFFFFFFFF) * (U64)((y) & 0xFFFFFFFF))
#endif

Where it's defined only checks for an _MSC_VER before using it. It would be very helpful if this could check architecture as well. Say either by not including it when ARM or only using it on x64/x86 processors. Thanks.

@Cyan4973 Cyan4973 added the bug label Jun 27, 2019
@Cyan4973
Copy link
Owner

Good point, thanks for notification @carstenskyboxlabs !

We should likely enforce that this code path is only valid for x86/x64, since that's the target where __emulu instruction is defined.

Side note : it would be great if I could find a way to test Visual Studio compilation for ARM, and automate such a test into AppveyorCI. It would have caught this issue, and potential future ones.

@carstenskyboxlabs
Copy link
Author

Thanks for the response.
I'm not sure I've never used AppveyorCI, but I do know that VS/MSBuild does support UWP ARM builds.

@CaptainCMake
Copy link

When I run these steps using CMake and Visual Studio 2019:

  1. cd cmake_unofficial
  2. mkdir build
  3. cd build
  4. cmake -G "Visual Studio 16 2019" -A ARM ..
  5. cmake --build .

I get this error:

xxhash.obj : error LNK2019: unresolved external symbol __emulu referenced in function XXH3_mul128_fold64 [C:\Users\steve\Do
cuments\xxHash\cmake_unofficial\build\xxhash.vcxproj]
C:\Users\steve\Documents\xxHash\cmake_unofficial\build\Debug\xxhash.dll : fatal error LNK1120: 1 unresolved externals [C:\U
sers\steve\Documents\xxHash\cmake_unofficial\build\xxhash.vcxproj]

I don't have much experience with appveyor.yml, but following the existing pattern, I think this will let you generate and run for ARM:

- if "%PLATFORM%"=="visual_arm" (
      cd cmake_unofficial &&
      cmake . -DCMAKE_BUILD_TYPE=Release -A ARM &&
      cmake --build . --config Release
  )

@Cyan4973
Copy link
Owner

Thanks @v-strob for the hint ! Worth trying !

@easyaspi314
Copy link
Contributor

This should probably work. It should fix ARM compilation, and it should improve x64 performance

#if defined(_MSC_VER) && defined(_M_IX86)
#    include <intrin.h>
#    define XXH_mult32to64 __emulu
#else
#    define XXH_mult32to64(x, y) ((U64)((x) & 0xFFFFFFFF) * (U64)((y) & 0xFFFFFFFF))
#endif

@Cyan4973
Copy link
Owner

Cyan4973 commented Jul 3, 2019

Fixed in dev branch

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

Successfully merging a pull request may close this issue.

4 participants