Remove force-sse2 config option from x86 builds. by Dead2 · Pull Request #1982 · zlib-ng/zlib-ng

Dead2 · 2025-10-10T11:30:46Z

Due to major refactoring done long ago, this option no longer avoids a branch in a hot path, it currently only removes a single if check during init.

Summary by CodeRabbit

Documentation
- Removed the FORCE_SSE2 entry from the Advanced Build Options table to reflect current build behavior.
Chores
- Removed the FORCE_SSE2 build/configure option and its associated CLI/help messaging.
- Disabled the ability to override SSE2 runtime checks via configure flags; default runtime detection is used.
Refactor
- Simplified SSE2 selection logic for 32-bit builds to rely on standard runtime detection.

coderabbitai · 2025-10-10T11:31:50Z

Walkthrough

Removes the FORCE_SSE2 build/configure option, its documentation entry, and any conditional -DX86_NOCHECK_SSE2 propagation; simplifies SSE2 runtime gating by removing X86_NOCHECK_SSE2 checks so runtime selection depends only on target width and cf.x86.has_sse2.

Changes

Cohort / File(s)	Summary
Build system: remove FORCE_SSE2 option and plumbing `CMakeLists.txt`, `configure`	Deleted `FORCE_SSE2` cmake_dependent_option, feature-info entry, configure `--force-sse2` handling, and conditional additions of `-DX86_NOCHECK_SSE2`.
Documentation: remove option from docs `README.md`	Removed the `FORCE_SSE2` row from the Advanced Build Options table.
Runtime/source gating: drop X86_NOCHECK_SSE2 checks `functable.c`, `arch/x86/x86_functions.h`	Removed `X86_NOCHECK_SSE2` from preprocessor guards; SSE2 fallback/selection now gated only by 64-bit detection and `cf.x86.has_sse2`/SSE2 defines.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant Init as Library Init
  participant CPU as CPU Feature Probe

  App->>Init: initialize()
  Init->>CPU: detect_features()
  CPU-->>Init: cf.x86.has_sse2

  alt 64-bit target
    Note over Init: SSE2 codepaths enabled (compile-time/layout unchanged)
    Init->>Init: select SSE2 implementations
  else 32-bit target
    alt cf.x86.has_sse2 == true
      Init->>Init: select SSE2 implementations
    else cf.x86.has_sse2 == false
      Init->>Init: select non-SSE2 implementations
    end
  end

  Note over Init: Removed build-time override path (FORCE_SSE2 / X86_NOCHECK_SSE2)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Fix incorrect declaration of FORCE_SSE2 #1880 — Modifies placement/handling of FORCE_SSE2 in CMake (directly related to removal/relocation of the same option).

Suggested labels

Build Env

Suggested reviewers

WildCard65
nmoinvaz

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately describes the primary change of removing the force-sse2 configuration option for x86 builds, directly reflecting the modifications to build scripts, documentation, and relevant source files.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch remove-force-sse2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d17069 and bbb4e33.

📒 Files selected for processing (5)

CMakeLists.txt (0 hunks)
README.md (0 hunks)
arch/x86/x86_functions.h (1 hunks)
configure (0 hunks)
functable.c (1 hunks)

💤 Files with no reviewable changes (3)

configure
README.md
CMakeLists.txt

🔇 Additional comments (2)

arch/x86/x86_functions.h (1)

94-94: LGTM! The obsolete X86_NOCHECK_SSE2 guard has been removed.

The preprocessor condition has been correctly updated. The SSE2 availability check now depends solely on:

X86_SSE2 (build system flag) AND __SSE2__ (compiler intrinsic support), OR

64-bit platform (__x86_64__ or _M_X64)

The removal of the || defined(X86_NOCHECK_SSE2) clause completes the elimination of the FORCE_SSE2 option, as flagged in the previous review. The change is correct and consistent with the PR objectives.

functable.c (1)

71-73: SSE2 runtime guard simplified; no X86_NOCHECK_SSE2 or FORCE_SSE2 remaining Verified via ripgrep: no matches found for X86_NOCHECK_SSE2 or FORCE_SSE2.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9ef3eb and 4d17069.

📒 Files selected for processing (4)

CMakeLists.txt (0 hunks)
README.md (0 hunks)
configure (0 hunks)
functable.c (1 hunks)

💤 Files with no reviewable changes (3)

README.md
CMakeLists.txt
configure

🧰 Additional context used

🧬 Code graph analysis (1)

functable.c (1)

arch/x86/chunkset_avx2.c (1)

loadchunk (44-46)

functable.c

Due to major refactoring done long ago, this option no longer avoids a branch in a hot path, it currently only removes a single if check during init.

codecov · 2025-10-10T15:35:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.93%. Comparing base (a9ef3eb) to head (bbb4e33).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #1982       +/-   ##
============================================
+ Coverage    39.28%   80.93%   +41.64%     
============================================
  Files           73      161       +88     
  Lines         7904    13741     +5837     
  Branches      1280     3121     +1841     
============================================
+ Hits          3105    11121     +8016     
+ Misses        4575     1610     -2965     
- Partials       224     1010      +786

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

nmoinvaz

Seems ok to me.

Dead2 added cleanup Improving maintainability or removing code. Architecture Architecture specific labels Oct 10, 2025

coderabbitai bot reviewed Oct 10, 2025

View reviewed changes

functable.c Show resolved Hide resolved

Remove force-sse2 config option from x86 builds.

bbb4e33

Due to major refactoring done long ago, this option no longer avoids a branch in a hot path, it currently only removes a single if check during init.

Dead2 force-pushed the remove-force-sse2 branch from 4d17069 to bbb4e33 Compare October 10, 2025 11:40

nmoinvaz approved these changes Oct 10, 2025

View reviewed changes

Dead2 merged commit 5159088 into develop Oct 11, 2025
304 checks passed

Dead2 mentioned this pull request Nov 5, 2025

2.3.0 RC1 #1994

Merged

coderabbitai bot mentioned this pull request Nov 12, 2025

Reorganize Chorba activation #2004

Merged

Dead2 deleted the remove-force-sse2 branch November 29, 2025 22:48

coderabbitai bot mentioned this pull request Dec 5, 2025

Improve CMake arch detection #2039

Merged

coderabbitai bot mentioned this pull request Dec 18, 2025

slide_hash_sse2 and slide_hash_avx2 are not dependent on HAVE_BUILTIN_CTZ #2051

Merged

coderabbitai bot mentioned this pull request Dec 30, 2025

Add support for trimming unused code based on compiler optimization for x86 #2066

Closed

coderabbitai bot mentioned this pull request Jan 8, 2026

Explicitly define the __SSE__ and __SSE2__ macros, since starting with MSVS 2012 the default instruction set is SSE2 #2076

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove force-sse2 config option from x86 builds.#1982

Remove force-sse2 config option from x86 builds.#1982
Dead2 merged 1 commit intodevelopfrom
remove-force-sse2

Dead2 commented Oct 10, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 10, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

codecov bot commented Oct 10, 2025 •

edited

Loading

Uh oh!

nmoinvaz left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Dead2 commented Oct 10, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nmoinvaz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Dead2 commented Oct 10, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 10, 2025 •

edited

Loading

codecov bot commented Oct 10, 2025 •

edited

Loading