WIP: fix(vk): keep TLS initialization coherent under COFF ThinLTO #755

Draft

siantm wants to merge 1 commit from siantm/Longshot:fix/coff-thinlto-tls into main

pull from: siantm/Longshot:fix/coff-thinlto-tls

merge into: MagicBeanDev:main

MagicBeanDev:main

Conversation 0 Commits 1 Files changed 7 +218 -16

siantm commented 2026-11-15 10:01:45 +00:00

First-time contributor

• Copy link
• Copy Markdown

Type: fix · Risk: medium · Tested on: Windows 11 Pro 26300, LLVM-MinGW 23.1.5, game-free · Needs: recorder-sync completion, full suite and Linux verification

Why

Windows COFF ThinLTO can fail to link the profiler and recorder's inline thread-local initialization routines. This fixes ownership and initialization without enabling ThinLTO or claiming an FPS improvement.

How to reproduce

Seen on: Windows 11 Pro 26300, LLVM-MinGW Clang 23.1.5 and lld.
Settings and switches: C++20 O2; native and -flto=thin, static runtime.
Steps: run python -I -B tests/test_thinlto_tls.py --cxx x86_64-w64-mingw32-clang++ --source-ref 9e6f231d, then the same command without --source-ref on this branch.
Expected: both consumers link and preserve cross-TU identity, per-thread separation and queued cleanup.
Actual: main's native arm succeeds; ThinLTO fails on initialization symbols for mkwvk::prof_t and mkwvk::rec::t_arena.

What changed

Constant-initialize profiler POD TLS and give the recorder arena function-local thread-local ownership. Migrate every arena consumer and preserve destruction-time draining.

• gpu/vk/vk_internal.h, gpu/vk/vk_recorder.h: focused production fix from local 073ed616.
• tests/test_thinlto_tls.py: compile real profiler/recorder consumers across translation units, with a baseline adapter.
• Recorder fixture callers and development documentation follow the accessor.

Measurements

Verdict: main fails during ThinLTO linking; the patched native and ThinLTO consumers pass.
Hardware: Windows 11 Pro 26300, Intel Core i7-9700, 31.8 GiB usable RAM; Python 3.14.7, LLVM-MinGW 23.1.5; no GPU used.
Method: the same authored cross-TU consumer against main and the prepared implementation.

Check	Before: main @ 9e6f231d	After: this PR @ 484daadbcd
Native consumer	Pass	Pass
COFF ThinLTO link	Undefined TLS initialization routines	Pass
Cross-TU/thread ownership and Enc exit drain	Native arm passes; ThinLTO cannot run	Native and ThinLTO pass

Observed patched output: PASS: native/ThinLTO production cross-TU TLS identity, thread isolation and Enc exit drain. Profiler sampling/exact-mode checks pass, as do recorder own/idle's 22 switch cases and deliberately broken variants. The checks ran on the source snapshot committed as 484daadbcd before browser submission.

Flags and settings

None. Production optimization defaults do not change; this does not enable the broader local ThinLTO experiment.

Testing

Tested: baseline/branch TLS comparison, profiler sampling/exact mode, recorder own/idle; run record accepted by the actual catalogue renderer.
Not tested: full game/renderer build, native Linux, full suite. The enclosing command timed out at 300 seconds while the larger test_recorder_sync.py driver was running; that driver is not reported as passed. The catalogue unit driver also has a Windows CRLF-vs-LF fixture failure.
Must be tested: complete recorder-sync and full suite on Windows; native Linux TLS/recorder checks before readiness, by hand or Test Farm. Investigate any timeout rather than treating it as success.

Risks

What could break: per-thread lifetime, initialization or destruction ordering.
Turning it off: revert this commit; no new toggle.
Compatibility: no save/settings/cache-format, renderer ABI or compiler-default changes.

Checklist

Everyone

• One topic.
• Conventional Commit title: fix(vk): keep TLS initialization coherent under COFF ThinLTO.
• Matching issue search completed; open PR titles reviewed, issue search remains pending.
• Measurements report the link-failure proof and limitations.
• Testing names tested, untested and required work.
• Recheck current main before publishing; prepared on 9e6f231d.
• Complete game-free suite; not run, recorder-sync timed out.
• No game files, dumps or build output.
• Third-party sources untouched.
• Existing Longshot source and authored fixtures; GPL-3.0-or-later.

When it applies

• Source commit records Assisted-by: OpenAI Codex.
• Measured run recorded under docs/tested/.
• Regression fails on main and passes here.
• Recorder consumer fixtures migrated without weakened assertions.
• Windows/Linux host verification complete; native Linux remains required.
• Development documentation updated.

**Type:** fix · **Risk:** medium · **Tested on:** Windows 11 Pro 26300, LLVM-MinGW 23.1.5, game-free · **Needs:** recorder-sync completion, full suite and Linux verification ## Why Windows COFF ThinLTO can fail to link the profiler and recorder's inline thread-local initialization routines. This fixes ownership and initialization without enabling ThinLTO or claiming an FPS improvement. ## How to reproduce **Seen on:** Windows 11 Pro 26300, LLVM-MinGW Clang 23.1.5 and lld. **Settings and switches:** C++20 O2; native and `-flto=thin`, static runtime. **Steps:** run `python -I -B tests/test_thinlto_tls.py --cxx x86_64-w64-mingw32-clang++ --source-ref 9e6f231d`, then the same command without `--source-ref` on this branch. **Expected:** both consumers link and preserve cross-TU identity, per-thread separation and queued cleanup. **Actual:** main's native arm succeeds; ThinLTO fails on initialization symbols for `mkwvk::prof_t` and `mkwvk::rec::t_arena`. ## What changed Constant-initialize profiler POD TLS and give the recorder arena function-local thread-local ownership. Migrate every arena consumer and preserve destruction-time draining. - `gpu/vk/vk_internal.h`, `gpu/vk/vk_recorder.h`: focused production fix from local 073ed616. - `tests/test_thinlto_tls.py`: compile real profiler/recorder consumers across translation units, with a baseline adapter. - Recorder fixture callers and development documentation follow the accessor. ## Measurements **Verdict:** main fails during ThinLTO linking; the patched native and ThinLTO consumers pass. Hardware: Windows 11 Pro 26300, Intel Core i7-9700, 31.8 GiB usable RAM; Python 3.14.7, LLVM-MinGW 23.1.5; no GPU used. Method: the same authored cross-TU consumer against main and the prepared implementation. | Check | Before: main @ 9e6f231d | After: this PR @ 484daadbcd | |---|---|---| | Native consumer | Pass | Pass | | COFF ThinLTO link | Undefined TLS initialization routines | Pass | | Cross-TU/thread ownership and Enc exit drain | Native arm passes; ThinLTO cannot run | Native and ThinLTO pass | Observed patched output: `PASS: native/ThinLTO production cross-TU TLS identity, thread isolation and Enc exit drain`. Profiler sampling/exact-mode checks pass, as do recorder own/idle's 22 switch cases and deliberately broken variants. The checks ran on the source snapshot committed as 484daadbcd before browser submission. ### Flags and settings None. Production optimization defaults do not change; this does not enable the broader local ThinLTO experiment. ## Testing **Tested:** baseline/branch TLS comparison, profiler sampling/exact mode, recorder own/idle; run record accepted by the actual catalogue renderer. **Not tested:** full game/renderer build, native Linux, full suite. The enclosing command timed out at 300 seconds while the larger `test_recorder_sync.py` driver was running; that driver is not reported as passed. The catalogue unit driver also has a Windows CRLF-vs-LF fixture failure. **Must be tested:** complete recorder-sync and full suite on Windows; native Linux TLS/recorder checks before readiness, by hand or Test Farm. Investigate any timeout rather than treating it as success. ## Risks **What could break:** per-thread lifetime, initialization or destruction ordering. **Turning it off:** revert this commit; no new toggle. **Compatibility:** no save/settings/cache-format, renderer ABI or compiler-default changes. ## Checklist ### Everyone - [x] One topic. - [x] Conventional Commit title: `fix(vk): keep TLS initialization coherent under COFF ThinLTO`. - [ ] Matching issue search completed; open PR titles reviewed, issue search remains pending. - [x] Measurements report the link-failure proof and limitations. - [x] Testing names tested, untested and required work. - [ ] Recheck current main before publishing; prepared on 9e6f231d. - [ ] Complete game-free suite; not run, recorder-sync timed out. - [x] No game files, dumps or build output. - [x] Third-party sources untouched. - [x] Existing Longshot source and authored fixtures; GPL-3.0-or-later. ### When it applies - [x] Source commit records `Assisted-by: OpenAI Codex`. - [x] Measured run recorded under docs/tested/. - [x] Regression fails on main and passes here. - [x] Recorder consumer fixtures migrated without weakened assertions. - [ ] Windows/Linux host verification complete; native Linux remains required. - [x] Development documentation updated.

siantm added 1 commit 2026-11-15 10:01:45 +00:00

fix(vk): keep TLS initialization coherent under COFF ThinLTO ... 484daadbcd

Focused extraction of local Longshot changes: 073ed616. Authored tests and verification limits included; no game-derived artifacts.

Assisted-by: OpenAI Codex

mooki added the

area/renderer

kind

bug

platform/linux

platform/windows

priority

high

labels 2026-11-15 13:59:25 +00:00

HumanBean force-pushed fix/coff-thinlto-tls from 484daadbcd to 4150b1f2e9 2026-11-15 18:45:01 +00:00 Compare

BeanIssues added the

risk

medium

label 2026-11-15 20:27:45 +00:00

Some checks failed

Hide checks

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped

Details

Third-party parts / The same as upstream (pull_request) Has been skipped

Details

Website / Website on Cloudflare (pull_request) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

CI / Conventional Commit title (pull_request) Successful in 3s

Details

CI / Workflow lint (actionlint) (pull_request) Successful in 5s

Details

Guide screenshots / Guide images and local UI source review (pull_request) Failing after 4s

Details

Third-party parts / As their notes say (pull_request) Successful in 8s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

This pull request is marked as a work in progress.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u 'fix/coff-thinlto-tls:siantm-fix/coff-thinlto-tls'

git switch 'siantm-fix/coff-thinlto-tls'

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area/accessibility

Barriers for people with disabilities

  

area/audio

Sound and music

  

area/input

Controllers, keyboard, mouse

  

area/install

Installing, updating, building from your game

  

area/menu

In-game menu and settings

  

area/renderer

Vulkan renderer, shaders, effects

  

area/saves

Save files

  

area/tooling

Developer tools: scripts, CI, build and release tooling

  

area/upscaling

DLSS, FSR, frame generation

  

beanstalk: approved @ 14e165a

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 1697603

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 2c5a62e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 4fd82b7

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 64d6d09

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 6f91fae

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 70a78f9

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 8015c9a

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 8d361dc

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ 9d8356e

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ abf2227

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ d82f202

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ e5f1e40

Set by Beanstalk for show; the farm's database is what counts

  

beanstalk: approved @ ee8ab5f

Set by Beanstalk for show; the farm's database is what counts

  

ci: approved

A maintainer approved CircleCI for this fork PR (sticky; removed after 14 quiet days)

  

good first issue

A good start for a new contributor

  

gpu/amd

AMD GPUs

  

gpu/igpu

Integrated GPUs

  

gpu/intel

Intel GPUs

  

gpu/nvidia

NVIDIA GPUs

  

help wanted

Extra hands or knowledge wanted

  

kind

bug

Something doesn't work as it should

  

kind

chore

Build, CI, dependencies, tidying

  

kind

docs

Documentation

  

kind

feature

A new feature or an improvement

  

kind

performance

Frame rate, frame time, loading, memory

  

kind

question

A question rather than a change

  

needs measurements

Waiting for before/after measurements

  

platform/linux

Affects Linux (a change for every system carries both Windows and Linux)

  

platform/macos

macOS (Apple silicon or Intel Macs)

  

platform/steam-deck

Steam Deck

  

platform/windows

Affects Windows (a change for every system carries both Windows and Linux)

  

preview

Maintainers: deploy a preview of this PR's website (read its changes first). Removed after the run.

  

priority

critical

Crashes, data loss, or blocks most players

  

priority

high

Important: next in line

  

priority

low

Nice to have

  

regression

Worked in an earlier build, broken in a newer one: highest priority, above other bugs

  

risk

high

Broad or core change: the renderer, defaults on many systems, saves, installs, security

  

risk

low

Small, contained change: little can break

  

risk

medium

Changes behaviour in one area, or a default on some systems

  

status

blocked

Waits on something else first

  

status

confirmed

Reproduced or agreed on

  

status

duplicate

Already reported or proposed

  

status

invalid

Not a problem in this project

  

status

needs-info

Waiting for more information from the reporter

  

status

wontfix

Will not be worked on

  

testing wanted

Wanted: a test on other hardware (posts a test request in #test-requests)

  

ci: approved

No labels

area/accessibility

area/audio

area/input

area/install

area/menu

area/renderer

area/saves

area/tooling

area/upscaling

beanstalk: approved @ 14e165a

beanstalk: approved @ 1697603

beanstalk: approved @ 2c5a62e

beanstalk: approved @ 4fd82b7

beanstalk: approved @ 64d6d09

beanstalk: approved @ 6f91fae

beanstalk: approved @ 70a78f9

beanstalk: approved @ 8015c9a

beanstalk: approved @ 8d361dc

beanstalk: approved @ 9d8356e

beanstalk: approved @ abf2227

beanstalk: approved @ d82f202

beanstalk: approved @ e5f1e40

beanstalk: approved @ ee8ab5f

ci: approved

good first issue

gpu/amd

gpu/igpu

gpu/intel

gpu/nvidia

help wanted

kind

bug

kind

chore

kind

docs

kind

feature

kind

performance

kind

question

needs measurements

platform/linux

platform/macos

platform/steam-deck

platform/windows

preview

priority

critical

priority

high

priority

low

regression

risk

high

risk

low

risk

medium

status

blocked

status

confirmed

status

duplicate

status

invalid

status

needs-info

status

wontfix

testing wanted

ci: approved

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

MagicBeanDev/Longshot!755

No description provided.