Repository navigation
Conversation
Dextroid17
left a comment
There was a problem hiding this comment.
I built this branch and ran the new focused test under the environment CI uses. On my machine it does not pass there, so I want to put the measurement in front of you before it reaches main.
Environment: CI's own settings (ANYPS5_REQUIRE_VULKAN=1, GALLIUM_OVERRIDE_CPU_CAPS=avx — .github/workflows/build.yml), lavapipe Mesa 26.2.4 / LLVM 23.1.1, CachyOS, Release build of this branch (f2205c2c).
caps=avx agc_driver_bda_device_tests --cross-range-only -> exit 139 (SIGSEGV) 5/5 runs
caps=sse2 ... -> exit 139
caps=sse4_2 ... -> exit 0
caps=avx2 ... -> exit 0
caps unset ... -> exit 0
It is deterministic here (5/5), and the unfiltered agc_driver_bda_device_tests segfaults the same way, so this reads as the pre-existing #1044 crash rather than something this diff introduces — this PR changes only the tests and the CMake file, and the driver is untouched. Which is also why I read "Fixes #1044" as optimistic: the cross-range read still faults, the test makes it focused and visible rather than fixed.
The fault is inside llvmpipe's JIT, on the wide probe path:
#0 0x00007ffff649687a in ?? ()
=> 0x7ffff649687a: vpbroadcastd (%rcx),%ymm3
A vector broadcast dereferencing a null base. With APS5_BDA_BYTE_READS set (either value) the same command exits 0, consistent with the byte-wise fallback avoiding that path — the same picture recorded on #1044.
Why it matters for CI: SKIP_RETURN_CODE 77 does not cover 139, so if CI's lavapipe reproduces this the linux job goes red, and --repeat until-fail:3 just repeats it three times.
Caveat in your favour: my Mesa is newer than CI's (apt-get install mesa-vulkan-drivers, so a distro build), and #1044 reports the case passing on Mesa 25.x. CI may well be fine. I cannot settle it from here, and the branch shows no CI run yet — so a CI run, or a run on the runner's lavapipe version, is the thing that decides it.
If it does reproduce, the options look like:
- land a driver-side avoidance first — take the byte-wise path whenever the wide one is unsafe — so the focused test has something to pass on; or
- have the focused target report 77 when the device cannot be exercised safely, rather than registering a test that cannot pass on that device.
Either way, "the focused test passes with lavapipe" is true only for caps that avoid the fault; worth naming which, since CI pins avx.
For what it is worth, making this a registered test is the right shape — I just do not think it can be green under CI's avx on a current Mesa, and I would rather you had the numbers than the red job.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
|
Following up on my review above, with something I only found afterwards that I think matters more than the caps detail. #1187 (johnmaia, opened 16:45 — earlier than this PR) registers the same test, and its approach to the lavapipe problem is the opposite of this one in a way worth comparing:
It also documents the same fault I hit: the third dispatch, a 32-bit read at So If that reading is right, the useful question is not whether this test can be made to pass, but whether this PR is still needed on top of #1187 — and if the focused, explicitly-registered cross-range case is wanted for its own sake, having it skip on a device that cannot run it (as #1187 does) rather than run it. Happy to be corrected — I have no title, and #1187's author has actual hardware (RADV and ANV) which I do not. |
|
Thanks for measuring this under CI's caps and for pointing out #1187. You're right that #1187 already registers the test, skips the BDA execution dispatches on CPU devices, and exits 77 without Vulkan, so it covers this without the duplication. Closing in favour of #1187. The cross-range case can come back as its own change once #1044 has a driver-side fix. |
The BDA backend already has a byte-wise fallback for reads spanning ranges, and the execution test contains the reported 8-bit, 16-bit, and 32-bit cases. This makes the exact cross-range regression a focused CTest test instead of leaving it inside an unregistered device-test executable.
The
--cross-range-onlymode runs the three boundary-read cases without the unrelated wide-vector and color-transfer checks. CTest builds and registers the focused target whenBUILD_TESTINGis enabled, and marks it skipped when the required Vulkan/BDA environment is unavailable.On this checkout, the focused test passes with lavapipe.
Fixes #1044