Skip to content

Build ICU with ClangCL so -march actually limits codegen#174

Merged
dylan-conway merged 2 commits intomainfrom
claude/icu-clang-cl
Mar 10, 2026
Merged

Build ICU with ClangCL so -march actually limits codegen#174
dylan-conway merged 2 commits intomainfrom
claude/icu-clang-cl

Conversation

@dylan-conway
Copy link
Copy Markdown
Member

MSVC /arch:SSE2 is a no-op on x64 and the auto-vectorizer emits AVX2/AVX512 regardless, so switch the stage 2 static rebuild to ClangCL and use /clang:-march= like we do for WebKit proper.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6ce5d77-68fe-426b-91a0-dfeace76e2c2

📥 Commits

Reviewing files that changed from the base of the PR and between ce9bf84 and 335daa8.

📒 Files selected for processing (1)
  • build-icu.ps1

Walkthrough

Modifies the build script to introduce ClangCL-compatible architecture flags for stage 2 builds, adds a new toolset argument configuration, and extends vcxproj patching logic to strip resource definitions and prevent rc.exe involvement during compilation.

Changes

Cohort / File(s) Summary
Build Script Enhancement
build-icu.ps1
Replaced x64 architecture flags with ClangCL-compatible flags (/clang:-march=nehalem or /clang:-march=haswell), introduced a new PlatformToolset argument for stage 2 MSBuild invocation, extended vcxproj patching to remove ResourceCompile blocks and related resource files, while preserving existing patch logic for DLL/StaticLibrary toggles and /GL disablement.
🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a clear technical rationale but does not follow the WebKit template structure (missing bug reference, reviewer line, detailed explanation section, and file listing). Add a Bugzilla bug reference link, follow the template format with 'Reviewed by' line, provide detailed explanation section, and list changed files with function/method breakdowns.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: switching ICU builds to ClangCL to make the -march flag effective for code generation limits.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@github-actions
Copy link
Copy Markdown

Preview Builds

Commit Release Date
335daa86 autobuild-preview-pr-174-335daa86 2026-03-10 05:47:12 UTC

@dylan-conway dylan-conway merged commit 9664a97 into main Mar 10, 2026
35 checks passed
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 this pull request may close these issues.

1 participant