Repository navigation
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
This PR fails two tests that pass on Built
Messages:
Both failures reproduce with For completeness: this machine has no read-write access to AI-assisted (Claude Code). |
|
Tested at
By commit:
The last case printed before the span failure is Minor: the debt entry is under "Unknown function info" rather than "Functional". AI-assisted: yes (Claude Code); results above were run on this machine. |
|
Rebase handoff — rebased onto current Verified on Linux (CachyOS, kernel 7.2.9, GCC 16.2.1, Ninja, Release, To pick it up: Head is |
|
One note on the handoff: |
|
Thanks for the rebase handoff and the follow-up that pins these failures to the PR’s own change. I confirm the two cases are still unresolved: |
|
Review status: the reported #7392 head |
|
Thanks for this. The branch conflicts with main only in One question on 1c5a918: the mixed valid/faulting barrier cases now run only when the device's native subgroup size is exactly 32, so they're also skipped on 64-wide hosts (RADV's default compute subgroup), not only on narrower ones. Is that intended, i.e. does the mixed layout assume 32 lanes? If they hold at 64, |
|
We reproduced both failures on an RTX 4090 with this branch on current main (main alone passes 16/16), and narrowed them down in case it helps the corrected series:
|
|
Thanks for reproducing this on the RTX 4090 and separating the two failures. The |
|
The mixed control expects valid lanes to write their two output dwords and faulting lanes to leave theirs unchanged; |
|
Thanks, these current-main repros are useful. The 32-only gate was conservative because the fixture uses a 32-lane guest wave; I do not have evidence that the mixed-lane case requires a native 32-lane subgroup, so I can’t justify claiming wave64 coverage from the current test. Separately, both |
|
Hey @MarcZX10, I’m starting a Windows repro of the two failures on Have you already started a corrected series? I’ll keep this work with #7392 rather than opening a competing PR, preserve the existing commit authors, and keep #6416 as the declared prerequisite. I also saw @Celegans12’s separate spanned-wave masking follow-up on #6416; I’ll flag any overlap before proposing changes to that path. No fix or game result is being claimed yet. AI-assisted investigation and independent review. |
|
I have not started a corrected series. Please continue the focused repro on this PR and keep #6416 as the prerequisite; I will avoid a competing fix while you investigate. The separate span-read call-count and wave64 fault-report paths should stay isolated as you described. |
|
Hey @MarcZX10, I reproduced both failures and have a separate test-only correction for each.
Tested against main
Each corrected test also passed five fresh-process runs on each GPU: 20/20 test executions, with all 12 fault controls showing zero wrong output dwords every run. Vulkan was required, the AnyPS5 shader cache was disabled, and the requested Khronos validation layer actually loaded; neither side reports a VUID. As a negative control, disabling Full Windows builds of pinned main, the original PR integration and the corrected integration passed with the same separate alignment workaround for Patch commits, authored as Blazing Cook:
The format-patches and combined diff apply cleanly to exact PR head The representative large-barrier compile-time check is still open. I couldn't locate a public replay attachment in the checked sources. Could someone share a reduced barrier+BDA replay request, or run an A/B on an existing request and report exact commits, GPU/driver, preparation time, SPIR-V words and cold pipeline compilation/cache controls? Native64, AMD, other platforms and title validation also remain open; no title-unblock or performance claim is made. At the final overlap check, Public log copies replace local filesystem prefixes with placeholders; the private originals are retained. The archive records public-entry hashes and raw/public provenance hashes. AI-assisted: yes. AnyPS5-7392-tests-and-evidence-20261021.zip |
|
@MarcZX10 a compile-time measurement for the open point in your Tested section ("Compile time for a representative very large barrier compute shader has not been measured"). For this shader the PR makes the back-end compile much faster, not slower. Shader. vkQuake (PPSA99010) compute
I only measured compile time and didn't check the PR's fault semantics. Replay tool, scripts and timings are on the M4 if you want any other variant measured. AI-assisted: yes |
|
Hey @Alexius0815, thanks for running this. The 69–79 s versus 27.5 s pipeline result is useful evidence for the open compile-time check. Could you share the retained replay request and timing scripts, or an original/shareable reduced request that exercises the same barrier+BDA path? To make a matched comparison, these details would help:
A shareable request would let us try the same input on our NVIDIA/Intel setup and compare with your Metal measurements. We'll keep the existing Windows fault-correctness results separate from compile performance. Your actual MoltenVK cache investigation already has an owner, so any help from us can stay coordinated here. AI-assisted: yes. |
|
Quick refresh on the two test corrections: they still fix both reported failures on main
Each corrected test also passed five fresh-process runs on each GPU: 20/20 test executions, with all 12 fault controls visible and no wrong output every run. Both GPUs reject the masking-off negative control: 8 barrier cases write incorrect output while every fault report stays visible. Production source bytes were restored and verified before the final rebuild/runs. All three source compositions passed the full default Windows build. Upstream now includes the alignment fixture change, so this refresh needs no local build workaround. Vulkan was required, the shader cache disabled and the requested Khronos validation layer actually loaded; no VUID/SYNC-HAZARD lines or skips in the focused runs. The full CTest suite was not run. Refreshed commits are At the final upstream check, main had advanced to AMD/native64, other platforms and title validation remain untested here. Alexius's Metal timing/replay follow-up is separate; these runs make no new compile-performance or title-unblock claim. Public log copies normalize local paths; raw originals are retained. AI-assisted: yes. AnyPS5-7392-refreshed-tests-and-evidence-20261022.zip |
|
@cookie42073-cmd thanks, here is the material. The attached
Origin. The kernel is PS5_vkQuake 1.0's compute shader Correction to my note above. "Before the PR, macOS never served this pipeline from its Metal cache" was wrong. Both arms come from the cache on a repeat. One repeat of the "before" MSL on the 22nd did miss it (78.8 s), and I don't know why. The PR doesn't change caching; the difference is the cold compile. Pipeline step, by method. The README explains each and gives library and specialization times, which don't change.
AI-assisted: yes |
|
@MarcZX10, about the overlap with #11255: this PR adds the "BDA read faults" entry directly above "VCC-backed MIMG samplers" in My proposal is no fixed order: whichever lands second rebases and keeps both. If this one lands first, I'll rebase #11255 myself. Does that work for you? |
|
Got the Windows compile-only comparison for @Alexius0815's supplied SPIR-V pair. Both files compile on the RTX 4070 SUPER. Neither controlled Intel UHD 770 replay finished within the same 300-second limit. Tool source: main RTX 4070 SUPER,
Order alternated before/after, after/before, before/after. The driver had already seen both modules in the pilots. Driver caches remained uncontrolled, and these figures do not establish cold-driver compilation cost or a current-PR speedup. Imported cache files were checked against their own GPU/module export; the tool reports no cache-hit feedback flag. Module, layout, replay-total and process times are separate in the reports. On Intel, the controlled before and after attempts each reached 300 seconds without emitting a timing report. A separate 20-second API-dump diagnostic on The initial pilots had recording/overlay hooks inserted and are retained separately: NVIDIA 37.621/34.647 seconds for pipeline creation, and an Intel-before replay timeout. They are excluded from the table. The controlled trials used per-child The exact supplied shader bytes and original format-39 request remain unchanged. These are derived supplemental wrappers: set 0 storage buffers 59/60/61, count 1; push constants 0..128 bytes; uint32 specialization IDs 8241/8242/8243 all 0; subgroup 32; pipeline flags 16; stage flags 0. Those settings are explicit assumptions/defaults, not the original macOS capture. This uses the supplied translations, rather than regenerating them from the unpublished integration. No shader dispatch, output correctness, gameplay, PS5, native Metal/MSL or CPU-preparation claim. No full default build/full CTest, AMD or other-platform run. Alexius supplied the artifacts; Celegans12, Amoenus and MarcZX10 retain implementation credit. I'm adding the Windows comparison. AI-assisted, with independent review of the raw results and final package. Text-only evidence and portable wrapper maker ZIP SHA-256: An Intel run of this same pair on another driver would help narrow the remaining limit; the useful details are the exact driver, exit/timeout and API-phase log. |
|
@Michael-KD that works for me: no fixed order, and whichever of #7392 and #11255 lands second rebases and keeps both entries in |
4b76e2c to
39a0161
Compare
❌ NVIDIA GPU test: failedPR head
Failing testsThis PR changes test files, so each failing test was also built and run on main with only the PR's test changes applied. ✅ there means the failure comes from the PR's other changes; ❌ means the test fails on main too, so this run does not show the PR caused it.
agc_driver_bda_span_reads: Failed (also failed when re-run alone), assertion and diagnosticsagc_driver_bda_span_reads_draw_pipeline: Failed (also failed when re-run alone), assertion and diagnosticsagc_shader_memory: Failed (also failed when re-run alone), assertion and diagnosticsUnstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (5, not counted against this PR)Validation IDs already present on main (4 IDs in 404 tests)Skipped (39)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
Quick follow-up on today's NVIDIA report: I rechecked the two test corrections on current main Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release, bundled SPIRV-Tools enabled:
The extra test in the PR arms is The first correction counts the fault-returning Each of the three regression tests also passed three fresh-process runs per GPU: 18/18 additional test executions. Each span fixture checked all 12 fault controls with zero wrong output dwords. With barrier write masking deliberately disabled, all three tests fail on both GPUs: every fault report is still observed, while eight barrier controls per span fixture write wrong output (128 dwords for all-fault cases, 64 for mixed). Exact production source bytes were restored, rebuilt, and the final 8/8 set passed again on both GPUs. The full default Windows build passed for main, the submitted PR integration, the corrected integration and the restored final tree. FFmpeg reused the pinned prebuilt cache. Vulkan was required, the application shader cache disabled, implicit layers excluded per child, and Khronos validation actually loaded. No VUID/SYNC errors or selected-test skips. The full CTest suite was not run. Refreshed test commits, authored as Blazing Cook:
@MarcZX10, could you take these two test corrections into this PR when convenient? Celegans12, Amoenus and you retain implementation credit. #6416 remains the prerequisite; its separate performance hold is not addressed by these tests. No native64, AMD, other-platform, title-unblock or performance claim. The supplied-SPIR-V compile measurements remain a separate result. AI-assisted: yes, with independent source and raw-log review. Public logs normalize local paths; private originals are retained. Final artifact hashes describe the restored arm; earlier-arm binaries were not retained. AnyPS5-7392-current-tests-20261029.zip ZIP SHA256: |
Self-review — test corrections remain off-headThe new GPU report shows the current tests failing and the two supplied test corrections passing, but those corrections are not part of published head The report says both GPUs reproduce the current test failures and pass the corrected set, with the negative control failing as intended. This supports bringing the two test-only corrections into the PR while retaining Blazing Cook's authorship. It does not establish that the current public head passes those tests. I did not run the GPU tests or modify this branch. Exact report and patch references. |
|
Hey @MarcZX10, I prepared a tested current-main integration for the emitter conflict, keeping the existing implementation and the two supplied test corrections. Pins: main On Windows 11 Pro 26300, WinLibs GCC 15.2.0 r7, Release with bundled SPIRV-Tools:
The same three test failures reproduce before correction: the shader-memory helper assertion and the direct/draw-pipeline span-report checks. All three corrected tests also passed three fresh-process runs per GPU (18 additional test executions). Disabling barrier write masking makes all three reject the result on each GPU: on each GPU, all 24 reports across the two span fixtures stay visible, while 16 controls write wrong output. After restoring exact production bytes and rebuilding, the final sets pass again. Main's new The test corrections are now Vulkan was required, the application shader cache disabled, implicit layers excluded, and Khronos validation actually loaded. No VUID/SYNC errors or selected-test skips were added. Stale installed-layer manifest lookups still emit loader errors, so the raw logs are not error-free. This is a local integration handoff, not a push to your branch or a separate PR. #6416 remains the prerequisite. Its Astro startup evidence request is still open; these checks do not resolve that performance question or establish a title unblock. AMD/native64 and other platforms remain untested here. AI-assisted, with independent review of the resolution, tests and evidence package. AnyPS5-7392-current-main-handoff-20261030.zip ZIP SHA256: |
|
@MarcZX10 the branch still has the tests that fail: the NVIDIA run on this head (29 Oct) shows |
CnRJay
left a comment
There was a problem hiding this comment.
1. Stacked and not rerun. This is stacked on #6416 and the body says the runtime tests were not rerun after the rebase onto main 3de84fa; only git diff --check and the conventions script were. The five-run GPU results live in issue #6689, on the pre-rebase tree. Please rerun the BDA span-read tests on the current head.
2. Compile-time and size cost is unmeasured. The body states the per-lane write mask "may affect compile time" for large barrier compute shaders and that no measurement was made. The mask is added to every write, atomic and export in a shader that has a barrier plus a BDA read, so SPIR-V size and variant compile time grow with the number of stores. Please give SPIR-V word counts and compile time for one large barrier shader before and after, and say whether shaders that have BDA reads but no fault possibility are affected.
3. Validation. The tests are BDA execution tests on two GPUs but no mention of running with VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation and comparing for new VUID- lines against main, which the shader recompiler rules require. A barrier inside divergent control flow is the kind of thing the layer and the SPIR-V validator complain about.
4. Negative control for the mixed case. RunFault builds a mixed valid/faulting input and requires that valid lanes' writes land; does the test fail if the write mask is dropped (faulting lanes writing garbage) or if the barrier is skipped for faulting lanes? The body claims both lanes are covered but does not say the test was shown failing.
MarcZX10
left a comment
There was a problem hiding this comment.
Replies checked — failing tests and four review points need answers, corrections not on branch
The 29 Oct NVIDIA run at this head reports three new failures, and the branch still conflicts with current main.
What I ran
- Live head
39a016143547fd5d5ae866a78622240f58b0dcfd(OPEN, unchanged from batch). gh apicomments, reviews and review comments on #7392, newer than 2026-10-30T01:44:46Z, excluding MarcZX10 and bots.gh pr view 7392 --json body,statusCheckRollup; the 2026-10-29 TechLevelZero comment.git cat-file,git merge-tree --write-tree upstream/main 39a01614,git grepfor the write-mask gate andRunFault.- Nothing built or run.
Results
| Check | Result | Note |
|---|---|---|
| Live head | matches batch | OPEN |
merge-tree vs upstream/main |
exit 1 | conflict in SpirvEmitter.cpp |
| NVIDIA run 2026-10-29 (third-party) | failed | 3 new: agc_shader_memory fails with production change; both agc_driver_bda_span_reads variants |
Correction commits 7664761651, f492498c |
not in this clone | not verified |
| Hosted windows, relinker, relinker-asan, conventions | SUCCESS | linux, register-context, write-watch SKIPPED |
Findings
- @oneandonlydean (2026-11-01 09:49 UTC) asks to bring the two test corrections and a main merge onto the branch. Answer needed. The corrections are not in this clone.
- @CnRJay (2026-11-01 19:59 UTC), point 1: runtime tests not rerun after the rebase. Confirmed in the body. Answer needed.
- Points 2 and 3: compile time not measured, and no validation-layer comparison, both confirmed in the body. Answer needed.
- Point 4: the mixed-lane negative control.
RunFaultis atBdaSpanReads.cppL160. Write masking is gated atSpirvEmitter.cppL329 and applied to guest-state writes atSpirvFlowEmitter.cppL827, so "every write, atomic and export" is only partly right. Whether the test fails without the mask is not shown. Answer needed. - @cookie42073-cmd (2026-10-31) offers an integration ZIP outside the branch. No question asked. Not downloaded.
Sources
Not verified
- The correction commits and the ZIP contents.
- The mixed-lane negative control, compile time, and any build or test result.
|
Quick update after #6416's spanning-wave follow-up: I checked this barrier change on that newer prerequisite, keeping the two existing test corrections. This is a local integration, There were two production conflicts: the combined emitter must keep #6416's spanning-wave eligibility while adding the barrier mask, and the byte-read path must keep its Windows 11 Pro 26300, GCC 15.2.0 Release, SPIRV-Tools enabled; Intel UHD 770 (32.0.101.7088) and RTX 4070 SUPER (621.01):
In the barrier-disabled control, all 24 fault reports stay visible per GPU, but 16 controls write wrong output. The corrected and restored runs retain all reports with no wrong output. The final 22 checks pass, with Khronos validation loaded and no VUID/SYNC errors or CTest skips. Expected BDA fault diagnostics remain. Full build and conventions pass. This adds current-prerequisite integration evidence to the existing handoff. The negative control drops the write mask; it does not deliberately skip a barrier. Large-shader word counts and compile timings still need separate evidence. No title unblock, Linux/macOS or native-64 mixed-barrier claim. AI-assisted integration, testing and independent review. |
…ater writes A recorded BDA fault (StopBdaInvocationIf) returned from the invocation. In programs whose invocations vote together the others then wait for it forever: with two lanes per invocation (wave64 on a 32-lane host) an execz, vccz or scc0 branch is taken only when all 32 invocations vote, and a waterfall that clears a lane from an SGPR mask only by that lane's own ballot never ends. The stop also ended the partner lane, which had not faulted. In programs with two lanes per invocation, ballots or shuffles (not pixel or mesh), a fault now sets a per-lane flag instead. The invocation keeps running and voting, and each later buffer, address, image, LDS or GDS write or atomic and each export of that lane is skipped. Other programs still stop.
39a0161 to
99a8be7
Compare
|
Hi Thanks for rebasing onto current main. A follow-up to our 1 Nov comment: the rebased head
One more point on the base. The first commit here, |
❌ NVIDIA GPU test: failedPR head
Failing testsThis PR changes test files, so each failing test was also built and run on main with only the PR's test changes applied. ✅ there means the failure comes from the PR's other changes; ❌ means the test fails on main too, so this run does not show the PR caused it.
agc_driver_bda_span_reads: Failed (also failed when re-run alone), assertion and diagnosticsagc_shader_memory: Failed (also failed when re-run alone), assertion and diagnosticsFailing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (13 IDs in 544 tests)Skipped (49)ReproduceAutomated build and test on a contributor's NVIDIA machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
|
I refreshed the existing #6416/#7392 integration on main Support branch, tested head In barrier compute mode, The new regression is
The failing controls are Tests ran on Windows 11 Pro Full logs, receipts, commands, attribution and limitations. Independent source, regression, overlap and runtime reviews passed. Conventions passed against both tested main and the later The support integration retains the newer #6416 spanning/first-fault behavior and both of my prior test corrections. It carries the existing authors' cherry-picked commits with credit preserved; the evidence includes the original-to-carried mapping. #6416 remains a prerequisite. The unchanged public #7392 head is still This fixes propagation of faults already reported by the formatted helper, including the tested invalid-format store. Unmapped stores through |
What
Keep compute invocations participating in
s_barrierafter a BDA read fault, while recording the fault per lane and suppressing that lane's later writes, atomics and exports. The checked BDA read result is propagated back to the correct lane half. The regression tests cover all-fault and mixed valid/faulting lanes around a barrier. Hull and mesh barrier handling remains outside this change and is recorded in shader technical debt.This is stacked on #6416. Its original fault-lane commit and the follow-up author's commit remain attributed to their respective authors.
Tested
1002:13c0), driver 32.0.21036.18, Windows 11 Enterprise build 26200.main(3de84fa1),git diff --checkandpython3 tools/check_conventions.py --base upstream/mainpass. The runtime tests were not rerun after the rebase.Checklist
main; no other open PR implements the barrier-write fix