Skip to content

build: disable LLVM unroll-add-parallel-reductions on Apple targets#51848

Merged
jkleinsc merged 1 commit into
mainfrom
sam/disable-unroll-parallel-reductions
Jun 2, 2026
Merged

build: disable LLVM unroll-add-parallel-reductions on Apple targets#51848
jkleinsc merged 1 commit into
mainfrom
sam/disable-unroll-parallel-reductions

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

LLVM's loop unroller miscompiles sub-form reductions when introducing parallel reduction phis (llvm/llvm-project#201065): the partial accumulators are recombined with alternating signs instead of being summed. Under ThinLTO + PGO this miscompiles simdutf's arm64 utf8_length_from_latin1 in darwin-arm64/mas-arm64 release builds, undercounting UTF-8 lengths for Latin-1 strings.

On Apple Silicon this manifests as (shipped in v42.3.1):

  • Buffer.byteLength() returning values 2 bytes short for one-byte strings >= 64 chars containing U+0080-U+00FF characters at affected offsets
  • silently truncated Buffer.from(string) / TextEncoder.encode() results
  • a heap buffer overflow followed by a CHECK crash in node::Utf8Value (e.g. fs.writeFileSync with such strings >= ~1KB)

LLVM only enables the parallel-reduction unrolling for Apple M-like CPUs (getAppleRuntimeUnrollPreferences), so only Apple targets are affected; linux-arm64 and win-arm64 target generic CPUs and cannot hit this. The flag is applied at link time, where ThinLTO backend codegen runs, and is inert on branches without macOS ThinLTO.

Validated on a local release build with the exact CI configuration (ThinLTO + Electron PGO profile): the crash and corruption repros disappear with the flag, the affected loop remains unrolled (single accumulator), and Speedometer 3.1 is unchanged (-0.84%, p=0.48, interleaved A/B).

This patch can be removed once the upstream fix (llvm/llvm-project#201066) ships in the bundled clang.

Notes: Fixed silent data truncation in Buffer/TextEncoder APIs and a crash in fs.writeFileSync with non-ASCII strings on Apple Silicon.

@MarshallOfSound MarshallOfSound requested a review from a team as a code owner June 2, 2026 17:02
@MarshallOfSound MarshallOfSound added semver/patch backwards-compatible bug fixes target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels Jun 2, 2026
@jkleinsc jkleinsc merged commit bdc9ee6 into main Jun 2, 2026
86 checks passed
@jkleinsc jkleinsc deleted the sam/disable-unroll-parallel-reductions branch June 2, 2026 20:57
@release-clerk

release-clerk Bot commented Jun 2, 2026

Copy link
Copy Markdown

Release Notes Persisted

Fixed silent data truncation in Buffer/TextEncoder APIs and a crash in fs.writeFileSync with non-ASCII strings on Apple Silicon.

@trop

trop Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #51849

@trop trop Bot added the in-flight/42-x-y label Jun 2, 2026
@trop trop Bot removed the target/42-x-y PR should also be added to the "42-x-y" branch. label Jun 2, 2026
@trop

trop Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "43-x-y", please check out #51850

@trop

trop Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "41-x-y", please check out #51851

@trop trop Bot added in-flight/43-x-y in-flight/41-x-y merged/43-x-y PR was merged to the "43-x-y" branch. and removed target/43-x-y PR should also be added to the "43-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. in-flight/43-x-y labels Jun 2, 2026
@trop trop Bot added merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. and removed in-flight/41-x-y in-flight/42-x-y labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants