Skip to content

Add support for Windows XP #6176

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

Closed
wants to merge 21 commits into from
Closed

Conversation

Sugar-Baby
Copy link
Contributor

@Sugar-Baby Sugar-Baby commented Jul 12, 2025

通过将src/platform.h.in中的SRW Lock替换为CRITICAL_SECTION,将ConditionalVariable通过Event模拟实现,修改CMakeLists,添加新的toolchain实现了ncnn在Windows XP上的运行。
成功跑通benchncnn和examples。
benchncnn指令参数:benchncnn 4 1 0
结果:
VirtualBox_XP_11_07_2025_23_47_29
VirtualBox_XP_11_07_2025_23_53_23
报告:
benchncnn.txt
examples报告:
test_results.log

将工作过程同步到了docs/developer-guide/build-ncnn-on-windows-xp.zh.md,以及个人博客。同时在docs/how-to-build/how-to-build.md中添加了对应的指导。

QQ:1436217185

@tencent-adm
Copy link
Member

tencent-adm commented Jul 12, 2025

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.03%. Comparing base (e2d93a4) to head (0162687).
Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #6176       +/-   ##
===========================================
+ Coverage   93.34%   95.03%    +1.69%     
===========================================
  Files         507      835      +328     
  Lines      129805   251811   +122006     
===========================================
+ Hits       121165   239321   +118156     
- Misses       8640    12490     +3850     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nihui nihui requested a review from Copilot July 12, 2025 06:55
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds compatibility support for Windows XP (x86) by introducing a new MinGW-w64 toolchain, replacing Windows Vista+ synchronization primitives with XP-compatible ones, and updating build scripts, documentation, and CI.

  • Introduces windows-xp.toolchain.cmake to target WinXP with appropriate flags and disabled instruction sets.
  • Replaces SRWLOCK/CONDITION_VARIABLE in platform.h.in with CRITICAL_SECTION and event-based logic.
  • Updates CMakeLists, documentation guides, and GitHub Actions workflow to include Windows XP build and verification steps.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
toolchains/windows-xp.toolchain.cmake New toolchain file configuring MinGW-w64 flags for XP support
src/platform.h.in Swaps SRW locks/conditions for CRITICAL_SECTION/event-based logic
src/CMakeLists.txt Adds _WIN32_WINNT and WINVER compile definitions for XP
docs/how-to-build/how-to-build.md Adds a section for building on Windows XP using MinGW-w64
docs/developer-guide/build-ncnn-on-windows-xp.zh.md New Chinese guide for building ncnn on Windows XP
.github/workflows/windows-xp.yml GitHub Actions job for XP compatibility build, test, and verification
Comments suppressed due to low confidence (3)

docs/how-to-build/how-to-build.md:19

  • [nitpick] Add a space between XP and (x86) in the TOC entry for consistency with the section header.
- [Build for Windows XP(x86) using MinGW-w64](#build-for-windows-xp-x86-using-mingw-w64)

.github/workflows/windows-xp.yml:73

  • The workflow is looking under tests\\Release, but MinGW Makefiles builds don't create a Release subfolder. Update the path to the actual build output directory (e.g., build-xp/tests).
        Get-ChildItem -Path "tests\Release" -Filter "*.exe" | ForEach-Object {

.github/workflows/windows-xp.yml:80

  • Similarly, no src\\Release directory is generated by MinGW Makefiles; adjust this path to where DLLs are actually output (e.g., build-xp).
        Get-ChildItem -Path "src\Release" -Filter "*.dll" | ForEach-Object {

Comment on lines +108 to +116
ConditionVariable() { event = CreateEvent(NULL, TRUE, FALSE, NULL); }
~ConditionVariable() { CloseHandle(event); }
void wait(Mutex& mutex) {
LeaveCriticalSection(&mutex.cs);
WaitForSingleObject(event, INFINITE);
EnterCriticalSection(&mutex.cs);
}
void broadcast() { SetEvent(event); ResetEvent(event); }
void signal() { SetEvent(event); ResetEvent(event); }
Copy link
Preview

Copilot AI Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConditionVariable implementation using a manual-reset event with immediate ResetEvent can miss wake-ups under contention. Consider using an auto-reset event or PulseEvent, and ensure the event remains signaled long enough for all waiting threads.

Suggested change
ConditionVariable() { event = CreateEvent(NULL, TRUE, FALSE, NULL); }
~ConditionVariable() { CloseHandle(event); }
void wait(Mutex& mutex) {
LeaveCriticalSection(&mutex.cs);
WaitForSingleObject(event, INFINITE);
EnterCriticalSection(&mutex.cs);
}
void broadcast() { SetEvent(event); ResetEvent(event); }
void signal() { SetEvent(event); ResetEvent(event); }
ConditionVariable() { event = CreateEvent(NULL, FALSE, FALSE, NULL); }
~ConditionVariable() { CloseHandle(event); }
void wait(Mutex& mutex) {
LeaveCriticalSection(&mutex.cs);
WaitForSingleObject(event, INFINITE);
EnterCriticalSection(&mutex.cs);
}
void broadcast() { while (WaitForSingleObject(event, 0) == WAIT_OBJECT_0) { SetEvent(event); } }
void signal() { SetEvent(event); }

Copilot uses AI. Check for mistakes.

Copy link
Member

@nihui nihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msvc compiler support is missing and there is v141_xp toolchain

Copy link

github-actions bot commented Jul 12, 2025

The binary size change of libncnn.so (bytes)

architecture base size pr size difference
x86_64 15602512 15614920 +12408 ⚠️
armhf 6611348 6615580 +4232 ⚠️
aarch64 9921904 9921904 0 😘

@Sugar-Baby Sugar-Baby changed the title Add support for Windows XP (x86) Add support for Windows XP Jul 12, 2025
@Sugar-Baby
Copy link
Contributor Author

@nihui There are two cancelled checks, but the ci files that calls for the outdated runners are not relevant with my fix. Should I leave them aside or call later runners?

@nihui
Copy link
Member

nihui commented Jul 12, 2025

@nihui There are two cancelled checks, but the ci files that calls for the outdated runners are not relevant with my fix. Should I leave them aside or call later runners?

Just modifying the runner os may not be enough, maybe it also requires installing the toolset of vs2015/2017.

Fix the ci problem in a new pull request if you are willing to do it.

@nihui
Copy link
Member

nihui commented Jul 12, 2025

it seems that ci tests does not work

Test project D:/a/ncnn/ncnn/build-xp-mingw
No tests were found!!!

@Sugar-Baby
Copy link
Contributor Author

it seems that ci tests does not work

Test project D:/a/ncnn/ncnn/build-xp-mingw
No tests were found!!!

Should be fixed now.

@Sugar-Baby
Copy link
Contributor Author

@nihui There are two cancelled checks, but the ci files that calls for the outdated runners are not relevant with my fix. Should I leave them aside or call later runners?

Just modifying the runner os may not be enough, maybe it also requires installing the toolset of vs2015/2017.

Fix the ci problem in a new pull request if you are willing to do it.

Maybe I'll try that later?
And I wish to know as for now, with only those checks cancelled, could this pr be considered passed?

@nihui
Copy link
Member

nihui commented Jul 12, 2025

it seems that ci tests does not work

Test project D:/a/ncnn/ncnn/build-xp-mingw
No tests were found!!!

Should be fixed now.

ah, ci log shows that avx/avx512 is enabled in both mingw and msvc build, which is not expected

2025-07-12T08:57:09.2691677Z CMake Warning:
2025-07-12T08:57:09.2692204Z   Ignoring extra path from command line:
2025-07-12T08:57:09.2692597Z 
2025-07-12T08:57:09.2692798Z    "D:/a/ncnn/ncnn/toolchains/windows-xp.toolchain.cmake"
2025-07-12T08:57:09.2693139Z 
2025-07-12T08:57:09.2693144Z 
2025-07-12T08:57:09.2693229Z CMake Warning:
2025-07-12T08:57:09.2693475Z   Ignoring extra path from command line:
2025-07-12T08:57:09.2693708Z 
2025-07-12T08:57:09.2693840Z    "../toolchains/windows-xp.toolchain.cmake"

@Sugar-Baby
Copy link
Contributor Author

it seems that ci tests does not work

Test project D:/a/ncnn/ncnn/build-xp-mingw
No tests were found!!!

Should be fixed now.

ah, ci log shows that avx/avx512 is enabled in both mingw and msvc build, which is not expected

2025-07-12T08:57:09.2691677Z CMake Warning:
2025-07-12T08:57:09.2692204Z   Ignoring extra path from command line:
2025-07-12T08:57:09.2692597Z 
2025-07-12T08:57:09.2692798Z    "D:/a/ncnn/ncnn/toolchains/windows-xp.toolchain.cmake"
2025-07-12T08:57:09.2693139Z 
2025-07-12T08:57:09.2693144Z 
2025-07-12T08:57:09.2693229Z CMake Warning:
2025-07-12T08:57:09.2693475Z   Ignoring extra path from command line:
2025-07-12T08:57:09.2693708Z 
2025-07-12T08:57:09.2693840Z    "../toolchains/windows-xp.toolchain.cmake"

Should be fixed.

mkdir build-xp-msvc
cd build-xp-msvc

cmake -T v143,host=x64 -A x64 -DNCNN_VULKAN=OFF -DNCNN_SIMPLEOCV=ON -DNCNN_BUILD_TESTS=ON -DNCNN_RUNTIME_CPU=OFF -DNCNN_AVX=OFF -DNCNN_AVX2=OFF -DNCNN_AVX512=OFF -DNCNN_FMA=OFF -DNCNN_F16C=OFF -DNCNN_XOP=OFF -DNCNN_SSE2=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>" ..
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v143? x64? I think it's v141_xp and Win32 as you set in toolchains/windows-xp-msvc.toolchain.cmake. Maybe use the toolchain here directly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, v143 is actually vs2022

@Sugar-Baby
Copy link
Contributor Author

Sugar-Baby commented Jul 12, 2025

@nihui Edited that.
As for now:
2 checks cancelled
4 checks failed but all for seeming unrelated issues (multi-head attention and vulkan stuff).
Is there any other issue?

@Sugar-Baby
Copy link
Contributor Author

可以用msvc-wine项目中的脚本,在windows上安装vs2017,然后安装v141_xp支持工具包

https://github.com/mstorsjo/msvc-wine?tab=readme-ov-file#does-it-run-on-windows

Also I don't quite understand this. You mean I should use in actual deploying or install v141_xp in ci? v141_xp should still be available in vs2022 environment.

@Sugar-Baby
Copy link
Contributor Author

@nihui Edited that. As for now: 2 checks cancelled 4 checks failed but all for seeming unrelated issues (multi-head attention and vulkan stuff). Is there any other issue?

What should I do for those failed checks

@nihui
Copy link
Member

nihui commented Jul 12, 2025

@nihui Edited that. As for now: 2 checks cancelled 4 checks failed but all for seeming unrelated issues (multi-head attention and vulkan stuff). Is there any other issue?

What should I do for those failed checks

ignore any CI failures that are not relevant to your changes

@nihui
Copy link
Member

nihui commented Jul 12, 2025

可以用msvc-wine项目中的脚本,在windows上安装vs2017,然后安装v141_xp支持工具包
https://github.com/mstorsjo/msvc-wine?tab=readme-ov-file#does-it-run-on-windows

Also I don't quite understand this. You mean I should use in actual deploying or install v141_xp in ci? v141_xp should still be available in vs2022 environment.

FYI there is no v141_xp toolset in windows-2022 environment, you have to install it in your workflow for any uses

@Sugar-Baby Sugar-Baby requested a review from nihui July 12, 2025 11:55
@nihui
Copy link
Member

nihui commented Jul 12, 2025

mingw build log shows that msvc is used actually

CMake Warning:
  Ignoring extra path from command line:

   "D:/a/ncnn/ncnn/toolchains/windows-xp.toolchain.cmake"


CMake Warning:
  Ignoring extra path from command line:

   "../toolchains/windows-xp.toolchain.cmake"


-- Building for: Visual Studio 17 2022
-- CMAKE_INSTALL_PREFIX = D:/a/ncnn/ncnn/build-xp-mingw/install
-- NCNN_VERSION_STRING = 1.0.20250712
-- The C compiler identification is MSVC 19.44.35211.0
-- The CXX compiler identification is MSVC 19.44.35211.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.44.35207/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features

@Sugar-Baby Sugar-Baby requested a review from nihui July 12, 2025 12:34
@nihui
Copy link
Member

nihui commented Jul 12, 2025

github hosted windows runner has 4 cpus, do -j 4

ref https://docs.github.com/en/actions/reference/github-hosted-runners-reference

@Sugar-Baby
Copy link
Contributor Author

Sugar-Baby commented Jul 12, 2025

@nihui Seems all done. Thank you very much for your help!

@SA-j00u
Copy link

SA-j00u commented Jul 15, 2025

@Sugar-Baby can you release prebuild binaries?

also x32 version is requested 🙃

Copilot AI added a commit that referenced this pull request Jul 24, 2025
Co-authored-by: Sugar-Baby <[email protected]>
Co-authored-by: AtomAlpaca <[email protected]>

Co-authored-by: nihui <[email protected]>
nihui pushed a commit that referenced this pull request Jul 24, 2025
Co-authored-by: Sugar-Baby <[email protected]>
Co-authored-by: AtomAlpaca <[email protected]>
@nihui
Copy link
Member

nihui commented Jul 24, 2025

merged in 4644540

@nihui
Copy link
Member

nihui commented Jul 24, 2025

Thanks for your contribution !

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

Successfully merging this pull request may close these issues.

6 participants