Repository navigation
fix(shader): wrap the v_readlane_b32 lane select at the wave size - #322
Merged
boykopovar merged 3 commits intoOct 3, 2026
Merged
Conversation
MixPrecision and SdwaDestination dispatch hand-assembled RDNA through the recompiler and compare every lane with a CPU model, but were never in ctest. Move them to tests/execution/, which is globbed into one test per file with the same target names, so new execution tests add a file and no CMake lines. They exit 77 (skipped) when the host has no Vulkan device; the Linux runner installs lavapipe and sets ANYPS5_REQUIRE_VULKAN=1 so they cannot skip there. Co-Authored-By: Claude <noreply@anthropic.com>
The WriteLane IR takes {value, lane, previous}, as the translation and the
read-lane eliminator use it, but the SPIR-V emitter compared the lane id with
the previous value and selected between the lane index and the value. Every
v_writelane therefore wrote its value to all lanes (or the lane index to the
lane whose old value happened to equal its id).
Co-Authored-By: Claude <noreply@anthropic.com>
The hardware reads the lane select modulo the wave size (S1[4:0] in wave32, S1[5:0] in wave64), as ReadLaneEliminator already assumes with selector % waveSize. On the single-subgroup path EmitReadLane passed the raw select to OpGroupNonUniformShuffle, which is undefined for an id at or above the subgroup size. It is now masked with WaveSize() - 1, like v_writelane_b32. The two-lane wave64 path already wrapped.
oneandonlydean
added a commit
to oneandonlydean/AnyPS5
that referenced
this pull request
Oct 2, 2026
This was referenced Oct 2, 2026
Merged
Merged
This was referenced Oct 2, 2026
9 tasks done
9 tasks done
oneandonlydean
added a commit
to oneandonlydean/AnyPS5
that referenced
this pull request
Oct 3, 2026
…oykopovar#304 boykopovar#305 boykopovar#306 boykopovar#322 boykopovar#409 boykopovar#416, boykopovar#276 boykopovar#277 boykopovar#275 boykopovar#307, shader ops boykopovar#226 boykopovar#255 boykopovar#260 boykopovar#280 boykopovar#308 boykopovar#309 boykopovar#311 boykopovar#318 boykopovar#320 boykopovar#324 boykopovar#326 boykopovar#379 boykopovar#384 boykopovar#399, boykopovar#261, kernel boykopovar#335 boykopovar#341 boykopovar#349 boykopovar#355, libs boykopovar#227 boykopovar#244 boykopovar#245 boykopovar#273 boykopovar#274 boykopovar#279 boykopovar#286 boykopovar#287 boykopovar#289 boykopovar#296 boykopovar#300 boykopovar#328 boykopovar#358 boykopovar#402) into astrobot Where our PR merged in a revised form, main's version is taken; where astrobot carries the same work plus later local commits, astrobot's code stays and main's additions appear once. Conflicts: - AvPlayer (Export.cpp both libs, include/AvPlayer.hpp, src/Player.cpp, src/Source.cpp, AvPlayerLibrary.cmake, CMakeLists.txt, SceTypes.hpp, tests/GuestAvPlayer.cpp): main's boykopovar#307 (our reviewed squash, which carries 9c8e7fd e9ea5c8 adcede3 fb4eeb9 953057c ac456ee). It links the shared ffmpeg-core target, so astrobot's second FFmpeg build goes: 3rdparty/FFmpeg submodule, core/libs/FFmpeg.cmake and src/Exports.cpp. - libScePngEnc: main's boykopovar#227 (supersedes OverkillLabs' encoder). - libScePad Export.cpp: main's boykopovar#287 tilt correction (supersedes 49d45f5). - libSceAudioPropagation Export.cpp: astrobot's object model, which defines every function boykopovar#289 declares as a stub. - libSceSaveData.native Export.cpp: both (SaveDataHoldWrites and boykopovar#289's unknown exports). - VideoOutDriver.cpp: astrobot's (2546bd1 counters around boykopovar#416's lastFlipVblank store, same value). tests/Flip.cpp: boykopovar#416's pacing test and boykopovar#417's after-GPU test both kept. - boykopovar#278 files (DisplayFormat, VulkanDevice, GpuColorTransfer, ResidentPresent, agc CMakeLists): main's comment-free boykopovar#278 plus astrobot's P5 present-direct (b29728d), which removed DetileImage, ResidentPresent.hpp and the recorder-test hook; the resident present test stays its own executable. - VulkanDevice.cpp/.hpp: astrobot's GDS, local memory probe, sample dumps and P5 code; boykopovar#303/boykopovar#409 comments as main has them; main's shader clock (boykopovar#324) and DepthSurface-free includes. - RequestSerializer.cpp: version stays 8 (6 mesh passthrough, 7 nonConstantImageOffsets, 8 astrobot's later field). boykopovar#303's auto-merged second write and read of nonConstantImageOffsets removed. - SpirvEmitter.hpp, SpirvEmitterState.hpp, Recompiler.hpp/.cpp, StageIO.hpp, SpirvOptimizer.cpp, InstructionTranslator.cpp, ShaderReplay.cpp: local fields kept, boykopovar#303/boykopovar#305 comments as main has them. - SpirvAluEmitterMath.cpp: v_readlane keeps the split-wave path and takes boykopovar#322's wrap at the wave size for the others. - SpirvEmitter.cpp: astrobot's single-lane layout check. - SpirvFlowEmitter.cpp: boykopovar#304 as merged (the second lane emits a shared value itself when the first lane defined nothing). - SpirvImageEmitter.cpp: main's Offset operand placed before MinLod. - boykopovar#304: main's WaveUniformValues (SpirvAnalysis.cpp) replaces astrobot's copy in SpirvWaveExchange.cpp. - Recorder.hpp/.cpp, Draw.hpp, Graphics Draw.cpp, ShaderResources.cpp: astrobot's boykopovar#276/boykopovar#277 with its later snapshot work (per-use budgets, stale-snapshot refresh, profile counters, boykopovar#410, boykopovar#424, fan GS); boykopovar#409/boykopovar#306 comments as main has them. - PacketMemory.cpp: boykopovar#409 as merged plus astrobot's two opt-outs. - tests: ShaderMemory.cpp takes boykopovar#303's and boykopovar#304's new tests; Recorder.cpp takes boykopovar#409's afterRecordedWorkTests and drops main's copies of the boykopovar#276/boykopovar#277 tests astrobot already has; Structurizer.cpp keeps boykopovar#413's case. boykopovar#143 (NGG passthrough) stays as on astrobot: upstream did not touch its files since the boykopovar#258 revert.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to #254, as discussed there: #254 masks the
v_writelane_b32lane select, and adriadam10 leftv_readlane_b32/Shufflefor this PR ("I leftv_readlane_b32/Shufflefor your follow-up, as you offered.").The hardware reads the
v_readlane_b32lane select modulo the wave size: S1[4:0] in wave32, S1[5:0] in wave64.ReadLaneEliminatoralready assumes this for constant lanes (selector % waveSize), and the two-lane wave64 path inSpirvValueEmitContext::Shufflealready wraps (it masks with 31 and picks the half from bit 5). The single-subgroup path did not:EmitReadLanepassed the raw select straight toOpGroupNonUniformShuffle, which is undefined for an id at or above the subgroup size. A wave32v_readlane_b32 s8, v4, 37emittedOpGroupNonUniformShuffle %uint %uint_3 %v4 %uint_37.Changes:
EmitReadLanemasks the lane select withWaveSize() - 1beforeShuffle, the same expressionEmitWriteLaneuses after fix(shader): select the written lane in v_writelane_b32 #254. On the two-lane path the extra AND is redundant and folds away.ReadLaneEliminatordoes not cover this: it only rewrites a read it can resolve to av_writelanechain or a unique lane value, so other constant lanes (and every SGPR-selected lane) still reach the emitter.v_readfirstlane_b32is not changed here. Its lane comes fromOpGroupNonUniformBallotFindLSB(orFirstLaneon the two-lane path), which is in range whenever a lane is active. With an empty EXEC it is undefined on the single-subgroup path and lane 31 on the two-lane path, while the hardware reads lane 0. That is a separate fix, which I'll send after this one.Test
agc_driver_read_lane, in the style of #254'sagc_driver_write_lane(words fromllvm-mc -mcpu=gfx1013): a 32-thread wave32 dispatch where every lane readsv_readlane_b32with an inline lane of 37, then with an SGPR lane of 69 taken fromv_readfirstlane_b32of a loaded value. Both must return lane 5's value, and the third result checks thev_readfirstlane_b32value. Like the write-lane test, it is skipped when the subgroup cannot hold a wave32 (lavapipe: 8).Tested
ctest: 100/100 pass, including the new test.OpGroupNonUniformShufflewith lane 37 (and the dynamic lane 69), so NVIDIA happens to wrap the out-of-range id itself; the result is still undefined by the spec. On a device that runs a wave32 program on a 64-wide subgroup (RADV without fix(agc): run wave32 compute programs on 32-wide subgroups #316), lane 37 is an invocation outside the 32-thread dispatch, so I expect the old code to fail there, but I have no AMD GPU to check it.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository