Merge Windows XP support from PRs #6176 and #6177 into unified implementation #6204

Copilot · 2025-07-24T06:32:40Z

This PR consolidates the Windows XP support contributions from PR #6176 (by @Sugar-Baby) and PR #6177 (by @AtomAlpaca) into a single, comprehensive implementation that combines the best aspects of both approaches.

Changes Made

Core Windows XP Compatibility

Threading Support: Modified src/platform.h.in to conditionally use CRITICAL_SECTION and Event-based synchronization for Windows XP instead of SRWLock/ConditionVariable
Environment Variables: Updated src/cpu.cpp to use XP-compatible _putenv() instead of _putenv_s() when targeting Windows XP
Test Compatibility: Fixed tests/test_cpu.cpp to support Windows XP minimum requirements

Build Infrastructure

Added comprehensive build support with three separate toolchains:

MinGW-w64: toolchains/windows-xp-mingw.toolchain.cmake - 32-bit MinGW targeting i686
Clang: toolchains/windows-xp-clang.toolchain.cmake - Clang with MinGW libraries
MSVC: toolchains/windows-xp-msvc.toolchain.cmake - Visual Studio v141_xp toolset

CI/CD Integration

Created three separate GitHub Actions workflows for comprehensive testing:

.github/workflows/windows-xp-mingw.yml
.github/workflows/windows-xp-clang.yml
.github/workflows/windows-xp-msvc.yml

Documentation

English: Enhanced docs/how-to-build/how-to-build.md with detailed Windows XP build instructions for all three toolchains
Chinese: Added docs/developer-guide/build-ncnn-on-windows-xp.zh.md with comprehensive setup guide

Technical Implementation Details

The Windows XP compatibility layer uses conditional compilation based on _WIN32_WINNT > _WIN32_WINNT_WINXP:

#if _WIN32_WINNT > _WIN32_WINNT_WINXP // Windows Vista and later
    // Use modern SRWLock and ConditionVariable
#else // Windows XP compatibility
    // Use CRITICAL_SECTION and dual Event objects
#endif

Key features:

Automatic fallback to XP-compatible APIs when targeting Windows XP
Dual-event ConditionVariable implementation for proper signal/broadcast semantics
Disabled modern CPU features (AVX, Vulkan) for older hardware compatibility
Static linking support for standalone executables

Build Example

# Using MinGW-w64
cmake -DCMAKE_TOOLCHAIN_FILE="../toolchains/windows-xp-mingw.toolchain.cmake" \
      -DNCNN_SIMPLEOCV=ON -DNCNN_AVX=OFF -DNCNN_VULKAN=OFF \
      .. -G "MinGW Makefiles"

Credits

This implementation preserves and combines contributions from both original PRs:

@Sugar-Baby (PR Add support for Windows XP #6176): Original Windows XP support implementation and Chinese documentation
@AtomAlpaca (PR Add support for Windows XP #6177): Enhanced multi-toolchain approach and comprehensive CI workflows

Fixes #6203.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

tencent-adm · 2025-07-24T06:32:57Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Co-authored-by: Sugar-Baby <[email protected]> Co-authored-by: AtomAlpaca <[email protected]> Co-authored-by: nihui <[email protected]>

codecov-commenter · 2025-07-24T06:52:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.93%. Comparing base (fe509e9) to head (71f5bda).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6204      +/-   ##
==========================================
- Coverage   96.17%   95.93%   -0.24%     
==========================================
  Files         835      832       -3     
  Lines      260827   264287    +3460     
==========================================
+ Hits       250843   253542    +2699     
- Misses       9984    10745     +761

☔ 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.

Copilot

Pull Request Overview

This PR consolidates Windows XP support from two separate pull requests into a unified implementation, providing comprehensive build toolchain support and runtime compatibility for legacy Windows systems. The implementation uses conditional compilation to automatically select XP-compatible APIs when targeting older Windows versions.

Key changes include:

Implementation of Windows XP-compatible threading primitives using CRITICAL_SECTION and Event objects instead of modern SRWLock/ConditionVariable APIs
Addition of three complete build toolchains (MinGW-w64, Clang, MSVC) with appropriate XP targeting configurations
Comprehensive CI/CD integration with dedicated workflows for each toolchain to ensure compatibility testing

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/platform.h.in`	Adds conditional threading implementation with XP-compatible CRITICAL_SECTION and dual-event ConditionVariable
`src/cpu.cpp`	Updates environment variable setting to use XP-compatible `_putenv()` instead of `_putenv_s()`
`tests/test_cpu.cpp`	Modifies minimum Windows version check from Win7 to WinXP for test compatibility
`toolchains/windows-xp-*.toolchain.cmake`	Three new CMake toolchain files for MinGW, Clang, and MSVC with XP-specific configurations
`.github/workflows/windows-xp-*.yml`	Three CI workflows for comprehensive testing across all supported toolchains
`docs/**`	Enhanced documentation with detailed build instructions and Chinese language guide

Copilot · 2025-07-24T07:07:29Z

src/platform.h.in

+        mutex.unlock();
+        HANDLE events[2] = { signal_event, broadcast_event };
+        WaitForMultipleObjects(2, events, FALSE, INFINITE); // Wait for either signal or broadcast
+        mutex.lock();
+    }
+    void broadcast() 
+    { 
+        SetEvent(broadcast_event); // Wake all threads
+        ResetEvent(broadcast_event); // Reset after waking all threads


The current implementation has a race condition. After WaitForMultipleObjects returns, there's no way to distinguish whether it was woken by signal_event or broadcast_event, and the broadcast_event is reset immediately after SetEvent, which could cause threads to miss the broadcast if they haven't reached WaitForMultipleObjects yet.

Suggested change

mutex.unlock();

HANDLE events[2] = { signal_event, broadcast_event };

WaitForMultipleObjects(2, events, FALSE, INFINITE); // Wait for either signal or broadcast

mutex.lock();

}

void broadcast()

{

SetEvent(broadcast_event); // Wake all threads

ResetEvent(broadcast_event); // Reset after waking all threads

{

// Increment wait count

AutoLock lock(internal_mutex);

++wait_count;

}

mutex.unlock();

HANDLE events[2] = { signal_event, broadcast_event };

WaitForMultipleObjects(2, events, FALSE, INFINITE); // Wait for either signal or broadcast

mutex.lock();

{

// Decrement wait count

AutoLock lock(internal_mutex);

--wait_count;

if (wait_count == 0)

{

ResetEvent(broadcast_event); // Reset broadcast_event only when all threads are woken

}

}

}

void broadcast()

{

AutoLock lock(internal_mutex);

if (wait_count > 0)

{

SetEvent(broadcast_event); // Wake all threads

}

Copilot · 2025-07-24T07:07:29Z

src/platform.h.in

+    void broadcast() 
+    { 
+        SetEvent(broadcast_event); // Wake all threads
+        ResetEvent(broadcast_event); // Reset after waking all threads


Resetting the broadcast_event immediately after SetEvent creates a race condition where threads that haven't yet reached WaitForMultipleObjects may miss the broadcast signal. The reset should happen only after ensuring all waiting threads have been woken up.

github-actions · 2025-07-24T07:11:59Z

The binary size change of libncnn.so (bytes)

architecture	base size	pr size
x86_64	15639200	15639200
armhf	6648076	6648076
aarch64	9987568	9987568

nihui

LGTM

Initial plan

849bda0

Copilot AI assigned Copilot and nihui Jul 24, 2025

Copilot started work on behalf of nihui July 24, 2025 06:32 View session

Add Windows XP support merging PRs #6176 and #6177

71f5bda

Co-authored-by: Sugar-Baby <[email protected]> Co-authored-by: AtomAlpaca <[email protected]> Co-authored-by: nihui <[email protected]>

Copilot AI changed the title ~~[WIP] merge Windows XP support PRs #6176 and #6177 into a single contribution~~ Merge Windows XP support from PRs #6176 and #6177 into unified implementation Jul 24, 2025

Copilot AI requested a review from nihui July 24, 2025 06:49

Copilot finished work on behalf of nihui July 24, 2025 06:49

github-actions bot added core test doc cmake labels Jul 24, 2025

nihui requested a review from Copilot July 24, 2025 07:06

Copilot AI reviewed Jul 24, 2025

View reviewed changes

nihui approved these changes Jul 24, 2025

View reviewed changes

nihui marked this pull request as ready for review July 24, 2025 07:43

nihui merged commit 4644540 into master Jul 24, 2025
105 of 211 checks passed

nihui deleted the copilot/fix-6203 branch July 25, 2025 06:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge Windows XP support from PRs #6176 and #6177 into unified implementation #6204

Merge Windows XP support from PRs #6176 and #6177 into unified implementation #6204

Uh oh!

Copilot AI commented Jul 24, 2025 •

edited

Loading

Uh oh!

tencent-adm commented Jul 24, 2025

Uh oh!

codecov-commenter commented Jul 24, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 24, 2025

Uh oh!

Copilot AI Jul 24, 2025

Uh oh!

github-actions bot commented Jul 24, 2025

Uh oh!

nihui left a comment

Uh oh!

Uh oh!

Uh oh!

Merge Windows XP support from PRs #6176 and #6177 into unified implementation #6204

Merge Windows XP support from PRs #6176 and #6177 into unified implementation #6204

Uh oh!

Conversation

Copilot AI commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Made

Core Windows XP Compatibility

Build Infrastructure

CI/CD Integration

Documentation

Technical Implementation Details

Build Example

Credits

Uh oh!

tencent-adm commented Jul 24, 2025

Uh oh!

codecov-commenter commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 24, 2025

Uh oh!

nihui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jul 24, 2025 •

edited

Loading

codecov-commenter commented Jul 24, 2025 •

edited

Loading