Repository navigation
Conversation
Contributor
Author
|
@Raghav314, coordinating the test-registration add/add with #9508. The changes are functionally independent. This PR fixes the signed constant type in the opt-in gather diagnostic; the overlap is the new My recommendation is to choose the order by actual review/CI readiness. #9508 already has a review approval on its current head Whichever lands second should retain both the GPU-heap registration and the two diagnostic registrations. Can we use that plan? If you already have a concrete ordering preference based on the remaining work, please let me know; I will record the agreed order in |
|
Windows check of head Setup: Windows 11 Pro (10.0.26200), WinLibs GCC 15.2.0 r7 UCRT, Release,
Both drivers accept the ill-typed AI-assisted: yes. |
Contributor
✅ NVIDIA GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validation IDs already present on main (9 IDs in 596 tests)Skipped (56)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. |
This branch has not been deployed
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
APS5_GATHER_CONSTbuilds a signed gather result from unsigned constants. For anivec4result this produces invalid SPIR-V: the constituents and the vector components have different types. UseConstantI32for signed results, preserving the input's 32-bit pattern. The unsigned and floating-point paths keep their existing conversions.Extend the existing synthetic
ImageSint8execution fixture with two separately registered modes (1and0xffffffff). Each checks signed, unsigned and floating-point gather results and the untouched output guard. The ordinary signed-texture test remains the control. The same fixture source builds on main without the production fix.This was found while using the diagnostic switch to investigate a rendering problem. It repairs the opt-in diagnostic; it does not establish or fix the cause of that rendering problem.
Tested
Windows 11 Pro 10.0.26200, MinGW GCC 15.2.0, Release/Ninja,
BUILD_TESTING=ON,ANYPS5_ENABLE_SPIRV_TOOLS=ON,ANYPS5_AGC_EXECUTION_TESTS=ImageSint8. Synthetic fixture only; no game input is needed.The identical test source and registrations were run against main
e647fe16fdeewith only the tests applied, then against38a33bc3568dwith the production fix:agc_driver_image_sint8(ordinary loads, samples/gather and table checks)agc_driver_image_sint8_gather_constant_oneagc_driver_image_sint8_gather_constant_all_bitsBoth main failures report
Expected Constituents to be scalars or vectors of the same type as Result Type components. They fail on the signed case before reaching the unsigned/float controls. With the fix, all 32 lanes and four components match, and the output guard is unchanged. The all-bits case checks0xfffffffffor both integer formats and the existing unsigned-to-float conversion (0x4f800000) for UNORM.Both runs used
ANYPS5_REQUIRE_VULKAN=1,ANYPS5_NO_SHADER_CACHE=1andVK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validationon the same NVIDIA GPU, withctest --test-dir build-gather -R '^agc_driver_image_sint8($|_gather_constant_one$|_gather_constant_all_bits$)' -V --no-tests=error. The diagnostic tests setAPS5_SPIRV_OPT=noneand their respectiveAPS5_GATHER_CONSTvalues through CTest. The requested validation layer and expected AGC module were observed loaded in every fixture process. Neither run contains a missing-layer message,VUID-error or synchronization hazard; no tests skipped.The affected target builds, and the configured project registration audit passes (2040 test executables, none newly unregistered). The complete title/body conventions check passes. This is focused Windows validation, not a full product or cross-vendor test run.
The required overlap scan had incomplete comparisons that were checked manually against its fetched heads. None carries this diagnostic fix. #9508 and its carriers (#7642, #10193, #13928, #13929) add an independent GPU-heap registration to the same new CMake fragment. That add/add text conflict needs both registrations preserved by whichever lands second; the diagnostic fix has no implementation dependency on them. The main-vs-PR conflicts reported for #907 and #5371 already exist without this change. The shader/AGC components are unchanged between the tested main and the scan's
7264864185f8snapshot.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)