MSVC support #7

Closed
opened 2026-07-03 21:31:02 -05:00 by cgutman · 6 comments
cgutman commented 2026-07-03 21:31:02 -05:00 (Migrated from github.com)

The current runtime dispatching code using __attribute__((target("options")) and __builtin_cpu_supports() only works on GCC/Clang. Moonlight uses a vendored version of the older nanors code which is patched to compile with MSVC. The new runtime dispatching and GFNI codepaths would be fantastic to integrate, but I don't want to carry an even larger diff downstream for MSVC support.

Are you planning to add MSVC support? If not, would you be opposed to a PR that did so?

The current runtime dispatching code using `__attribute__((target("options"))` and `__builtin_cpu_supports()` only works on GCC/Clang. Moonlight uses a [vendored version](https://github.com/moonlight-stream/moonlight-common-c/tree/master/nanors) of the older `nanors` code which is patched to compile with MSVC. The new runtime dispatching and GFNI codepaths would be fantastic to integrate, but I don't want to carry an even larger diff downstream for MSVC support. Are you planning to add MSVC support? If not, would you be opposed to a PR that did so?
sleepybishop commented 2026-07-03 21:47:56 -05:00 (Migrated from github.com)

Ah, sorry I broke you guys, i just opened, #8 , WDYT?

Ah, sorry I broke you guys, i just opened, #8 , WDYT?
cgutman commented 2026-07-04 12:18:02 -05:00 (Migrated from github.com)

Thanks for taking care of this. Are you able to address the remaining few warnings?

rs.c(165): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data
deps/obl/oblas_lite.c(413): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)'
deps/obl/oblas_lite.c(451): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)'
deps/obl/oblas_lite.c(457): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)'
deps/obl/oblas16.c(20): warning C4146: unary minus operator applied to unsigned type, result still unsigned

I can suppress them manually in my project files if not.

Thanks for taking care of this. Are you able to address the remaining few warnings? ``` rs.c(165): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data deps/obl/oblas_lite.c(413): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)' deps/obl/oblas_lite.c(451): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)' deps/obl/oblas_lite.c(457): warning C4113: 'void (__cdecl *)(u8 *restrict ,u8 *restrict ,u8,unsigned int)' differs in parameter lists from 'void (__cdecl *)(uint8_t *,uint8_t *,uint8_t,unsigned int)' deps/obl/oblas16.c(20): warning C4146: unary minus operator applied to unsigned type, result still unsigned ``` I can suppress them manually in my project files if not.
sleepybishop commented 2026-07-04 12:24:16 -05:00 (Migrated from github.com)

Sure, I'll take a look

Sure, I'll take a look
sleepybishop commented 2026-07-04 12:38:31 -05:00 (Migrated from github.com)

i just pushed #10, PTAL

i just pushed #10, PTAL
cgutman commented 2026-07-04 13:07:06 -05:00 (Migrated from github.com)

The warning fix LGTM.

I was testing locally and found that some of the #ifdef guards are overly strict. MSVC only defines __AVX512F__ or __AVX2__ when those ISA extensions are guaranteed to be present (code built with /arch:AVX512 or /arch:AVX2). MSVC allows use of those extensions any time x86 is targeted, so I don't think any check is required.

There was a similar issue with the NEON codepath, but I needed to also adjust the initializers in obl_axpyb32_neon to make it build. I'm pretty sure the change is incompatible with GCC/Clang, so it'll need more changes to conditionally pick which initialization method to use.

I attached my patch. Feel free to use it as a reference for your changes.
msvc.patch

The warning fix LGTM. I was testing locally and found that some of the `#ifdef` guards are overly strict. MSVC only defines `__AVX512F__` or `__AVX2__` when those ISA extensions are _guaranteed_ to be present (code built with `/arch:AVX512` or `/arch:AVX2`). MSVC allows use of those extensions any time x86 is targeted, so I don't think any check is required. There was a similar issue with the NEON codepath, but I needed to also adjust the initializers in `obl_axpyb32_neon` to make it build. I'm pretty sure the change is incompatible with GCC/Clang, so it'll need more changes to conditionally pick which initialization method to use. I attached my patch. Feel free to use it as a reference for your changes. [msvc.patch](https://github.com/user-attachments/files/29663549/msvc.patch)
sleepybishop commented 2026-07-04 14:07:31 -05:00 (Migrated from github.com)

merged, thanks for your feedback.

merged, thanks for your feedback.
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-sleepybishop/nanors#7
No description provided.