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

MSVC support #17

Closed
AlyoshaVasilieva opened this issue Oct 18, 2020 · 12 comments · Fixed by #79
Closed

MSVC support #17

AlyoshaVasilieva opened this issue Oct 18, 2020 · 12 comments · Fixed by #79

Comments

@AlyoshaVasilieva
Copy link

Output of cargo run --release in a project using md5-asm via md-5:

   Compiling md5-asm v0.4.3
The following warnings were emitted during compilation:

warning: cl : Command line warning D9024 : unrecognized source file type 'src/x64.S', object file assumed
warning: cl : Command line warning D9027 : source file 'src/x64.S' ignored

error: failed to run custom build command for `md5-asm v0.4.3`

Caused by:
  process didn't exit successfully: `K:\Code\project\target\release\build\md5-asm-2f85d6e6f786c14b\build-script-build` (exit code: 1)
  --- stdout
  TARGET = Some("x86_64-pc-windows-msvc")
  OPT_LEVEL = Some("3")
  HOST = Some("x86_64-pc-windows-msvc")
  CC_x86_64-pc-windows-msvc = None
  CC_x86_64_pc_windows_msvc = None
  HOST_CC = None
  CC = None
  CFLAGS_x86_64-pc-windows-msvc = None
  CFLAGS_x86_64_pc_windows_msvc = None
  HOST_CFLAGS = None
  CFLAGS = None
  CRATE_CC_NO_DEFAULTS = None
  CARGO_CFG_TARGET_FEATURE = Some("fxsr,sse,sse2")
  DEBUG = Some("false")
  running: "K:\\Code\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.27.29110\\bin\\HostX64\\x64\\cl.exe" "-nologo" "-MD" "-O2" "-Brepro" "-W4" "-c" "-FoK:\\Code\\project\\target\\release\\build\\md5-asm-86902935336c4579\\out\\src/x64.o" "-c" "src/x64.S"
  cargo:warning=cl : Command line warning D9024 : unrecognized source file type 'src/x64.S', object file assumed
  cargo:warning=cl : Command line warning D9027 : source file 'src/x64.S' ignored
  cl : Command line warning D9021 : no action performed
  exit code: 0
  AR_x86_64-pc-windows-msvc = None
  AR_x86_64_pc_windows_msvc = None
  HOST_AR = None
  AR = None
  running: "K:\\Code\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.27.29110\\bin\\HostX64\\x64\\lib.exe" "-out:K:\\Code\\project\\target\\release\\build\\md5-asm-86902935336c4579\\out\\libmd5.a" "-nologo" "K:\\Code\\project\\target\\release\\build\\md5-asm-86902935336c4579\\out\\src/x64.o"
  LINK : fatal error LNK1181: cannot open input file 'K:\Code\project\target\release\build\md5-asm-86902935336c4579\out\src\x64.o'
  exit code: 1181

  --- stderr


  error occurred: Command "K:\\Code\\Microsoft Visual Studio\\2019\\Community\\VC\\Tools\\MSVC\\14.27.29110\\bin\\HostX64\\x64\\lib.exe" "-out:K:\\Code\\project\\target\\release\\build\\md5-asm-86902935336c4579\\out\\libmd5.a" "-nologo" "K:\\Code\\project\\target\\release\\build\\md5-asm-86902935336c4579\\out\\src/x64.o" with args "lib.exe" did not execute successfully (status code exit code: 1181).

Based on the .S files I don't think this project intends to support Windows. In that case it'd be nice if the project just gave a compile error saying Windows isn't supported rather than a confusing MSVC failure log.

@newpavlov
Copy link
Member

newpavlov commented Oct 19, 2020

Hm, I am not familiar with the MSVC build system, but I would expect it should be able to compile S files. Have you tried the GNU toolchain?

@tarcieri
Copy link
Member

@newpavlov it might be good to move this repo over to GitHub Actions and test on the windows-latest platform

@tarcieri
Copy link
Member

After doing a GitHub Actions conversion, I opened this PR to test Windows w\ MSVC: #19

It appears to have reproduced the issue:

https://github.com/RustCrypto/asm-hashes/pull/19/checks?check_run_id=1281176873

@tarcieri
Copy link
Member

We merged #19 which adds Windows 64-bit tests for MinGW.

If anyone would like to work on MSVC support, it should be easy to modify to test on that as well.

@not-an-aardvark
Copy link

not-an-aardvark commented Sep 9, 2021

I looked into this a bit while investigating the best approach for RustCrypto/hashes#315. My understanding is that there are two reasons MSVC builds are currently failing:

  1. The build scripts use the cc crate, which ostensibly invokes a C/C++ compiler, to compile an assembly file, even though the assembly file is not C/C++. This happens to work on other platforms, but apparently not on MSVC.
  2. Windows MSVC has a different calling convention from OSX/Linux, and the assembly will get called according to the native calling convention due to the use of extern "C". In other words, even if the assembly file was built correctly, the output would presumably be incorrect because all the arguments would start out in the wrong registers. I think maybe this could be fixed by using extern "sysv64" instead, but this purported fix would be difficult to verify until the build script issue is addressed.

@not-an-aardvark
Copy link

Based on the cc documentation, it seems like it does support assembly files, but it expects them to have the file extension .asm rather than .S on MSVC. So maybe having the build script copy x64.S to x64.asm would address the issue.

I don't have a windows or MSVC machine to test this on. I suppose I can try testing it on GitHub Actions.

@not-an-aardvark
Copy link

Sadly, this still doesn't work because the assembler on MSVC accepts an assembly file using MASM syntax rather than GAS syntax, so it's necessary to rewrite the whole assembly file to convert it to the other syntax. I think I got maybe 70% of the way there manually, for the sha1 implementation, but there are still some assembler errors and I'm out of time to work on this for now.

It's also possible that there are other alternatives than rewriting all of the assembly, such as using a different assembler or auto-converting things, but I'm not sure.

@tarcieri
Copy link
Member

One of the main alternatives we're looking at here is Rust intrinsics:

#41 (comment)

It seems like intrinsics can provide mostly equivalent performance but in a much more portable way.

It'd be great if MSVC users could chime in with reports about how the new intrinsics implementation is working out for them.

@not-an-aardvark
Copy link

That sounds great, although IIUC the portability of the intrinsics will also be limited because the relevant x64 intrinsics are only present on intel processors manufactured in the last five years or so.

@tarcieri
Copy link
Member

We do runtime detection to detect if CPUs are compatible, and fall back on a portable reference implementaton

@newpavlov
Copy link
Member

@not-an-aardvark
They are also present on relatively resent AMD CPUs as well.

@tarcieri
Copy link
Member

RustCrypto/hashes#312 added an AVX2-based backend using stdarch intrinsics. It doesn't rely on "SHA-NI" and is as fast as the assembly version.

zyluo added a commit to zyluo/asm-hashes that referenced this issue May 16, 2023
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 16, 2023
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 16, 2023
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 16, 2023
Hashes implented in MASM inspired by Project Nayuki
Requires Build Tools for Visual Studio to cargo build
https://visualstudio.microsoft.com/thank-you-downloading-visual-studio/?sku=BuildTools&rel=17

Fixes RustCrypto#17
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 17, 2023
Hashes implented in MASM inspired by Project Nayuki
Requires Build Tools for Visual Studio to cargo build
https://visualstudio.microsoft.com/thank-you-downloading-visual-studio/?sku=BuildTools&rel=17

Fixes RustCrypto#17
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 17, 2023
Hashes implented in MASM inspired by Project Nayuki
Requires Build Tools for Visual Studio to cargo build
https://visualstudio.microsoft.com/thank-you-downloading-visual-studio/?sku=BuildTools&rel=17

Fixes RustCrypto#17
zyluo added a commit to zyluo/asm-hashes that referenced this issue May 17, 2023
Hashes implented in MASM inspired by Project Nayuki
Requires Build Tools for Visual Studio to cargo build
https://visualstudio.microsoft.com/thank-you-downloading-visual-studio/?sku=BuildTools&rel=17

Fixes RustCrypto#17
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

Successfully merging a pull request may close this issue.

4 participants