Repository navigation
test(agc): align the indirect arguments block at run time - #6452
oneandonlydean merged 1 commit into
Conversation
|
Windows check. Windows 11, WinLibs GCC 15.2.0 r7 (MinGW), Release,
AI-assisted: yes (Claude Code) |
jotitadev
left a comment
There was a problem hiding this comment.
Static note from macOS, head 4fe92c9c2: this isn't Windows-only. Apple's linker doesn't reject alignas(65536) on a static object, it lowers it with a warning, so main's fixture can get a block that is not 64 KiB aligned there without any build error.
I compiled a minimal copy of the fixture's globals (alignas(256) array, alignas(65536) std::array<std::uint8_t, 65536> Block{}, alignas(256) array) with Apple clang from Xcode 27 on an M2 Max, macOS 27.2:
ld: warning: reducing alignment of section __DATA,__common from 0x10000 to 0x1000 because it exceeds segment maximum alignment (x86_64)
ld: warning: reducing alignment of section __DATA,__common from 0x10000 to 0x4000 because it exceeds segment maximum alignment (arm64)
x86_64 -O0: Block=0x102208000 mod 64K = 0x8000
arm64 -O0: Block=0x102df8000 mod 64K = 0x8000
x86_64/arm64 -O2: mod 64K = 0 (by chance of layout)
So on Mach-O the alignment depends on the layout. The aligned span inside a 2 * BlockBytes - 1 array, plus the new runtime Require, fixes it there too. The PR title could say "on Windows and macOS", or just "align the indirect arguments block at run time". #6478 uses std::aligned_alloc off Windows, which also gets it right on macOS.
I didn't build or run this fixture. The results above come from the standalone snippet only.
Written with AI assistance (Claude Code). I compiled and ran the snippet above on my machine.
WinLibs GCC 15.2.0 rejects the alignas(65536) static array that agc_driver_image_array_window_past_layer_limit added in #6772 (requested alignment '65536' exceeds object file maximum 8192), which stops a Windows build with every AGC execution test. Take a 64 KiB aligned 4 MiB span of a larger static array instead, as #6452 did for the indirect arguments block, and check its alignment at start.
What
Fixes #6716. Restore the Windows build of
agc_driver_gpu_indirect_arguments_misaligned_writer_ahead, added by merged #5747. The required WinLibs GCC 15.2.0 r7 rejects itsalignas(65536)static object:Select an aligned 64 KiB span inside a zero-initialized
2 * BlockBytes - 1backing array. The storage lasts for the whole process and always has room for the aligned span. An explicit runtime assertion checks alignment. The registered range remains 64 KiB;ArgumentsOffset=4, shaders and the 24-group/two-adopted-dispatch assertions are unchanged. No production driver changes.Reported on the original PR in this comment. No overlapping open fix was found at preparation time; later alternatives are tracked below.
Tested
Windows 11 Enterprise 26200, WinLibs GCC 15.2.0 r7, Release/bundled dependencies. Head
4fe92c9c23c7bd861d63664d4efde3033b6226f6, main0029e62f974c453a5f6c7333f3bdb13e770e3903; live main at readiness check0029e62f974c453a5f6c7333f3bdb13e770e3903.0c4816f61: compiling this fixture's object target fails with the diagnostic above. Its source is byte-identical at main0029. This is a compiler regression control, before GPU execution.libsbuilds, conventions and test-registration checkers pass, with all execution fixtures built.CastAddressToIntegerAtReturnportability advisory in unchangedLow(), which deliberately packs the low 32 address bits for the guest descriptor; its body is byte-identical to main. No diagnostics are suppressed and neither analyzer reports parser/configuration failures.The original campaign above did not run Linux. The current-main Linux execution refresh is recorded below; no title-startup, performance or new full Linux application result is claimed.
Independent Windows confirmation from @johnmaia (source): required WinLibs GCC 15.2.0 r7, Release, BUILD_TESTING=ON, main0029 versus this PR merged into that main. Main fails with the same alignment diagnostic; the corrected target builds and its fixture passes on NVIDIA RTX 4080 /617.42 and Intel UHD 770 /101.6129. These are the other contributor's target/fixture results, with no repetition counts or full-application test claim; our own repeated runs above remain RTX 5080 and AMD Radeon Graphics.
Checklist
Portability review and alternative correction
@jotitadev's macOS review independently ran a standalone globals snippet, not this fixture: Xcode 27 Apple clang, M2 Max, macOS 27.2. Apple's linker lowered 64 KiB section alignment to 4 KiB (x86_64) / 16 KiB (arm64); both O0 snippets produced a block at offset 0x8000 modulo 64 KiB. O2 happened to align it. The span plus runtime assertion addresses that layout dependency too. This is attributed snippet evidence, not a full macOS AnyPS5 build or GPU run; the source head is unchanged. The title now describes the general alignment correction.
Later-opened #6478 proposed an alternative correction of this same fixture using VirtualAlloc/aligned_alloc. Its author confirmed consolidation here and closed #6478. The overlap is resolved. This PR retains the static backing and runtime alignment assertion, full Windows build/regression, two-vendor repetition, and attributed independent RTX 4080 / Intel UHD 770 fixture checks above. Our #6498 validates with this PR's exact correction as its separate Windows build dependency.
Current-main portability and consolidation (2026-10-16 UTC)
Composition 861ad99e9 is main5d plus this unchanged head. The fixture source is byte-identical to4fe. Selected-target builds and the original test pass5/5 per RTX5080/Radeon integrated GPU on Windows11/GCC15.2r7, and5/5 on Ubuntu24/WSL2/GCC13.3/Mesa25.2.8 llvmpipe. Both device selections are checked per process; maximum1.12s/0.49s/0.21s, no skips. Conventions pass on both hosts. These are selected-target checks, not new full application builds; the earlier full Windows and static-analysis results retain their original pins. Detailed response and consolidation request.
Later-opened #6726 proposes dynamic allocation for this same declaration/failure; its author requested consolidation guidance. I compared its633078d1e head and recommend retaining this smaller static-span correction, as already done for closed #6478. #6716 is the same reported build failure. #6726 is now closed; the duplicate overlap is resolved.
Current Windows CI coverage: main93's successful job builds this target from
tests/ExecutionTestNotBuilt.cpp, then CTest skips it. The workflow selects only three real execution fixtures. Thus that green job does not compile or validate the real over-aligned declaration addressed here. The actual-fixture local and independent validation above remains the relevant evidence; this log inspection is not a fresh current-main compile.