From a7848f0a03186d9b5353a83c3b3b23704eed2ee3 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 23 Sep 2026 16:41:46 -0400 Subject: [PATCH] [MG_Backend, MG_IntegrationTest, scripts] (P7): an invalid or empty image unit on the wire arm binds a null storage descriptor (VK_EXT_robustness2 nullDescriptor), else a placeholder private to its (binding, unit) --- .../DirectVulkan/Renderer/UniformManager.cpp | 16 +- .../DirectVulkan/Renderer/UniformManager.h | 37 ++++- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 38 +++++ .../DirectVulkan/Renderer/VulkanRenderer.h | 3 + .../Renderer/WirePlaceholderImages.inc | 37 ++++- MobileGL/MG_IntegrationTest/CMakeLists.txt | 28 +++- .../Scenarios/InvalidImageUnitScenario.cpp | 145 +++++++++++++++++- scripts/ci/spawn_lane_parity.py | 6 +- 8 files changed, 295 insertions(+), 15 deletions(-) diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp index 4a231834..1368d6ab 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.cpp @@ -111,8 +111,10 @@ namespace MobileGL::MG_Backend::DirectVulkan { // (level, layer) the unit names is INVALID. A load returns zero, a store does nothing, an // atomic updates nothing - and none of it is an error, neither at bind time (the bind only // checks the level's and the layer's signs) nor at the draw. It is the observable an empty - // unit has, so the answer is the empty unit's: the storage placeholder, cleared before each - // use, of the dimensionality the SHADER declared (ResolveWirePlaceholderImage). + // unit has, so the answer is the empty unit's (ResolveWirePlaceholderImage): a NULL storage + // descriptor where the device enables VK_EXT_robustness2 nullDescriptor, else a storage + // placeholder private to the (binding, unit), cleared before each use, of the dimensionality + // the SHADER declared. // // KHR-GL46.shader_image_load_store.incomplete_textures is the shape that found it: level 2 // of a texture that defines only level 0 (MAX_LEVEL 7, a mipmapping filter). The same @@ -165,8 +167,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (unit < 0 || static_cast(unit) >= (storage ? MG_Pipe::kMGPipeMaxImageUnits : MG_Pipe::kMGPipeMaxTextureUnits)) return false; const auto handle = storage ? state.BoundShaderImages[unit].Res : state.BoundSamplerViews[unit].Texture; + // An EMPTY image unit is the same 8.26 invalid access as the named-but-missing texel + // below, so it takes the same answer: a null storage descriptor where the device has + // one, else the (binding, unit)-private placeholder (codex closeout finding 2). if (MG_Pipe::MGPipeHandleIsNull(handle)) - return ResolveWirePlaceholderImage(commandBuffer, program, programObj, binding, storage, out); + return ResolveWirePlaceholderImage(commandBuffer, program, programObj, binding, storage, out, + VK_FORMAT_UNDEFINED, static_cast(unit)); if (handle.Slot >= state.TextureResources.size()) WireDescriptorFatal("image-record"); const auto& record = state.TextureResources[handle.Slot]; if (!record.Live || record.Gen != handle.Gen) WireDescriptorFatal("image-record-generation"); @@ -175,7 +181,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { if (storage && WireShaderImageNamesNoTexel(*m_textureManager, state.BoundShaderImages[unit])) { const VkFormat bound = MG_Util::ConvertTextureInternalFormatToVkEnum( MG_Util::ConvertGLEnumToTextureInternalFormat(state.BoundShaderImages[unit].InternalFormat)); - return ResolveWirePlaceholderImage(commandBuffer, program, programObj, binding, storage, out, bound); + return ResolveWirePlaceholderImage(commandBuffer, program, programObj, binding, storage, out, bound, + static_cast(unit)); } auto* resource = m_textureManager->SyncTextureResourceByHandle(handle, false, storage); if (!resource) WireDescriptorFatal("image-resource"); @@ -652,6 +659,7 @@ namespace MobileGL::MG_Backend::DirectVulkan { #if MOBILEGL_BUILD_DISAGGREGATED for (auto& entry : m_wirePlaceholderImages) DestroyWirePlaceholderImage(entry.second); m_wirePlaceholderImages.clear(); + m_wireInvalidStorageImagesBindNull = false; #endif m_bufferManager = nullptr; diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h index 275a3664..a50f739c 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/UniformManager.h @@ -61,6 +61,16 @@ namespace MobileGL::MG_Backend::DirectVulkan { static constexpr Uint32 kWireDescriptorSetBudget = 2048; Bool WireDescriptorSetBudgetReached(Uint32 frameIndex) const; SizeT RewindWireDescriptorSets(Uint32 frameIndex); + // How the wire arm answers a STORAGE image unit that GL 4.6 core 8.26 makes invalid (no + // texture, or a (level, layer) its texture lacks): a load returns zero, a store and an + // atomic are discarded. `deviceNullDescriptor` is the renderer's enablement of + // VK_EXT_robustness2's nullDescriptor at device creation. With it the unit binds a NULL + // storage-image descriptor, which is exactly that rule. Without it (or under + // MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1, the host lanes' way onto this arm) the + // unit binds a placeholder PRIVATE to its (binding, unit), cleared before each use, so no + // two units ever alias; what that cannot give is a load of the SAME unit after its own + // store inside one pass reading zero (CONTRACT-P7 §12). Called after Initialize. + void SetWireInvalidStorageImageArm(Bool deviceNullDescriptor); #endif // A command buffer (re)began recording: descriptor bindings recorded into // the previous buffer do not carry over, so drop the bind-dedup shadow. @@ -210,13 +220,34 @@ namespace MobileGL::MG_Backend::DirectVulkan { VkImageLayout layout = VK_IMAGE_LAYOUT_UNDEFINED; Uint32 layers = 1; }; - mutable UnorderedMap m_wirePlaceholderImages; + // The placeholder's shape (format, target, storage, shadow) and, for a STORAGE placeholder + // on the arm without a null descriptor, the (binding, unit) it is private to. A sampled + // placeholder is never written, so it stays shared: its binding and unit are ~0u. + struct WirePlaceholderKey { + Uint64 shape = 0; + Uint32 binding = ~0u; + Uint32 unit = ~0u; + Bool operator==(const WirePlaceholderKey& other) const { + return shape == other.shape && binding == other.binding && unit == other.unit; + } + }; + struct WirePlaceholderKeyHash { + SizeT operator()(const WirePlaceholderKey& key) const { + Uint64 h = key.shape * 0x9E3779B97F4A7C15ull; + h ^= (static_cast(key.binding) << 32 | key.unit) + 0x9E3779B97F4A7C15ull + (h << 6) + (h >> 2); + return static_cast(h); + } + }; + mutable UnorderedMap m_wirePlaceholderImages; + // See SetWireInvalidStorageImageArm. + Bool m_wireInvalidStorageImagesBindNull = false; // boundStorageFormat: for a storage binding with no reflected format, the format the // unit's glBindImageTexture named, when the unit holds a texture it cannot address - // (GL 4.6 core 8.26); UNDEFINED keeps the numeric-domain R32 default. + // (GL 4.6 core 8.26); UNDEFINED keeps the numeric-domain R32 default. `unit` is the image + // unit of a storage binding (the key of its private placeholder); unused for a sampler. Bool ResolveWirePlaceholderImage(VkCommandBuffer commandBuffer, const MagmaProgramSource& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, Bool storage, - VkDescriptorImageInfo& out, VkFormat boundStorageFormat = VK_FORMAT_UNDEFINED) const; + VkDescriptorImageInfo& out, VkFormat boundStorageFormat = VK_FORMAT_UNDEFINED, Uint32 unit = ~0u) const; void DestroyWirePlaceholderImage(WirePlaceholderImage& image) const; #endif diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index fa325ee2..8277fc75 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3409,6 +3409,9 @@ void main() { m_physicalDevice.properties.limits.minUniformBufferOffsetAlignment, m_config.MaxFramesInFlight, maxProgramBindings, kDescriptorSetsPerFrame, m_textureManager.get(), m_samplerManager.get()); MOBILEGL_ASSERT(succeeded, "UniformDescriptorBinder initialization failed."); +#if MOBILEGL_BUILD_DISAGGREGATED + m_uniformManager->SetWireInvalidStorageImageArm(m_wireNullDescriptor); +#endif #if MOBILEGL_PIPE_PUSH m_vertexInputStateFactory = MakeUnique(m_config, m_physicalDevice.handle, m_pipeIdentity); @@ -15626,6 +15629,41 @@ void main() { } } +#if MOBILEGL_BUILD_DISAGGREGATED + // Codex closeout finding 2 (cf-magma): GL 4.6 core 8.26 makes an access through an + // invalid image unit load zero and DISCARD stores and atomics. A null storage-image + // descriptor is that rule exactly, so the wire arm binds one for such a unit + // (UniformManager::SetWireInvalidStorageImageArm). ONLY nullDescriptor is requested: + // robustBufferAccess2 / robustImageAccess2 would put a bounds check on every access of + // every draw. Wire arms only, on the same terms as the depth-resolve extensions above - + // the monolith arm resolves image units elsewhere and keeps its device as it was. + m_wireNullDescriptor = false; + VkPhysicalDeviceRobustness2FeaturesEXT robustness2Features{}; + robustness2Features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_ROBUSTNESS_2_FEATURES_EXT; + const char* robustness2Name = + IsExtensionSupported(availableExtensions, VK_EXT_ROBUSTNESS_2_EXTENSION_NAME) ? VK_EXT_ROBUSTNESS_2_EXTENSION_NAME + : IsExtensionSupported(availableExtensions, VK_KHR_ROBUSTNESS_2_EXTENSION_NAME) ? VK_KHR_ROBUSTNESS_2_EXTENSION_NAME + : nullptr; + if (MG_Config::Transport != MG_Config::TransportMode::Monolith && robustness2Name != nullptr && + getPhysicalDeviceFeatures2 != nullptr) { + VkPhysicalDeviceFeatures2 featureQuery{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2}; + featureQuery.pNext = &robustness2Features; + getPhysicalDeviceFeatures2(m_physicalDevice.handle, &featureQuery); + if (robustness2Features.nullDescriptor == VK_TRUE) { + EnableOptionalDeviceExtension(availableExtensions, enabledDeviceExtensions, robustness2Name); + robustness2Features.robustBufferAccess2 = VK_FALSE; + robustness2Features.robustImageAccess2 = VK_FALSE; + robustness2Features.pNext = const_cast(deviceCreateInfo.pNext); + deviceCreateInfo.pNext = &robustness2Features; + m_wireNullDescriptor = true; + } + } + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + MGLOG_I("Magma wire: robustness2 nullDescriptor %s (%s)", m_wireNullDescriptor ? "enabled" : "unavailable", + robustness2Name != nullptr ? robustness2Name : "no robustness2 extension"); + } +#endif + deviceCreateInfo.enabledExtensionCount = static_cast(enabledDeviceExtensions.size()); deviceCreateInfo.ppEnabledExtensionNames = enabledDeviceExtensions.data(); MGLOG_I("Device feature support: robustBufferAccess=%s geometryShader=%s independentBlend=%s logicOp=%s shaderClipDistance=%s " diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 55b9d52f..aefcb7ae 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -556,6 +556,9 @@ namespace MobileGL::MG_Backend::DirectVulkan { WireMultisampleResolveResources* m_wireMultisampleResolveResources = nullptr; void DestroyWireMultisampleResolveResources(); Bool m_wireShaderStencilExport = false; + // VK_EXT_robustness2 (or its KHR promotion) with ONLY nullDescriptor enabled, wire arms + // only: an invalid image unit binds a null storage descriptor (codex closeout finding 2). + Bool m_wireNullDescriptor = false; // The resolve probe's verdict for this device (WireDepthResolveArm.h, // WireDepthResolveProbe.h): true when the no-draw render pass was measured leaving its // target unwritten while the shader control resolved - the shader arm then resolves diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/WirePlaceholderImages.inc b/MobileGL/MG_Backend/DirectVulkan/Renderer/WirePlaceholderImages.inc index 397d9b58..14fce8dd 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/WirePlaceholderImages.inc +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/WirePlaceholderImages.inc @@ -13,9 +13,28 @@ image = {}; } + void UniformManager::SetWireInvalidStorageImageArm(Bool deviceNullDescriptor) { + const char* forced = std::getenv("MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER"); + const Bool forcePrivate = forced != nullptr && std::strcmp(forced, "1") == 0; + m_wireInvalidStorageImagesBindNull = deviceNullDescriptor && !forcePrivate; + MGLOG_I("Magma wire: an invalid or unbound image unit binds %s (VK_EXT_robustness2 nullDescriptor %s%s)", + m_wireInvalidStorageImagesBindNull ? "a NULL storage descriptor" + : "a placeholder private to its (binding, unit)", + deviceNullDescriptor ? "enabled" : "unavailable", + deviceNullDescriptor && forcePrivate ? "; MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1" : ""); + } + Bool UniformManager::ResolveWirePlaceholderImage(VkCommandBuffer commands, const MagmaProgramSource& program, const ProgramFactory::VkProgramObject& programObj, Uint32 binding, Bool storage, - VkDescriptorImageInfo& out, VkFormat boundStorageFormat) const { + VkDescriptorImageInfo& out, VkFormat boundStorageFormat, Uint32 unit) const { + // GL 4.6 core 8.26, codex closeout finding 2: every access through an invalid image unit + // is invalid - a load returns zero, a store and an atomic are discarded. A NULL storage + // descriptor (VK_EXT_robustness2 nullDescriptor) is exactly that, for any target, format + // and sample count, so nothing below is needed. + if (storage && m_wireInvalidStorageImagesBindNull) { + out = {VK_NULL_HANDLE, VK_NULL_HANDLE, VK_IMAGE_LAYOUT_GENERAL}; + return true; + } if (binding >= programObj.samplerTextureTargetByBinding.size() || binding >= programObj.samplerNumericDomainByBinding.size()) return false; const auto target = programObj.samplerTextureTargetByBinding[binding]; @@ -74,8 +93,18 @@ break; default: return false; } - const Uint64 key = static_cast(format) | (static_cast(target) << 32) | - (static_cast(storage) << 48) | (static_cast(shadow) << 49); + // A STORAGE placeholder is private to its (binding, unit). Shared by shape alone, a store + // through invalid unit A was visible, after a barrier, to a load through invalid unit B of + // the same shape - one Vulkan image behind both (codex closeout finding 2). The clear + // below still runs before each use; what a private image cannot hide is the SAME unit's + // own store to its own later load inside one pass (CONTRACT-P7 §12). + WirePlaceholderKey key; + key.shape = static_cast(format) | (static_cast(target) << 32) | + (static_cast(storage) << 48) | (static_cast(shadow) << 49); + if (storage) { + key.binding = binding; + key.unit = unit; + } auto found = m_wirePlaceholderImages.find(key); if (found == m_wirePlaceholderImages.end()) { WirePlaceholderImage image{}; @@ -169,7 +198,7 @@ auto& image = found->second; // Sampled incomplete textures remain immutable opaque black. A storage // placeholder is reset before each use, so a prior discarded store cannot - // leak into an unrelated invocation's unbound image loads. + // leak into a later pass's loads through the same (binding, unit). if (storage || image.layout == VK_IMAGE_LAYOUT_UNDEFINED) { VkClearValue clear{}; if (!storage) { diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index eaeb2db3..233012f0 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -3506,7 +3506,9 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ALevelPastAnImmutableTexturesStorageReadsZero ALayerPastAnArraysLastSliceReadsZero ATextureWithNoStorageReadsZero - ALevelPastATextureViewsWindowReadsZero) + ALevelPastATextureViewsWindowReadsZero + AStoreThroughAnInvalidUnitIsLoadedThroughNeitherAnotherInvalidUnitNorItself + AStoreThroughAnEmptyUnitIsLoadedThroughNeitherAnotherEmptyUnitNorItself) gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectVulkan.${mglItestArm}.ImageUnitWindow." TEST_FILTER "InvalidImageUnitScenario.${imageWindowCase}" @@ -3515,6 +3517,30 @@ if (MOBILEGL_BUILD_DISAGGREGATED) ${mglItestArmProperties} ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-imagewindow${mglItestArmLogTag}-${imageWindowCase}.log") endforeach() + # ---- Codex closeout finding 2 (cf-magma): THE ARM WITHOUT A NULL DESCRIPTOR ----------- + # + # The two AStoreThrough... cases above run the null-descriptor arm on this host: lavapipe + # enables VK_EXT_robustness2 nullDescriptor, so an invalid or empty storage unit binds a + # null descriptor. A device without it binds a placeholder PRIVATE to the (binding, unit) + # instead; MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1 puts the server on that arm + # (UniformManager::SetWireInvalidStorageImageArm), and the client reads the same marker to + # expect it: B's load zero (no alias), A's own load of its store = the recorded CONTRACT-P7 + # §12 residual, which is also what proves the knob reached the server. + # + # Split+spawn only, the server-side-knob reason above, named in spawn_lane_parity.py's + # MAGMA_SERVER_ENV_KNOB_NO_TCP. + if(mglItestServerEnvKnobArm) + foreach(privateCase AStoreThroughAnInvalidUnitIsLoadedThroughNeitherAnotherInvalidUnitNorItself + AStoreThroughAnEmptyUnitIsLoadedThroughNeitherAnotherEmptyUnitNorItself) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.${mglItestArm}.ImageUnitPrivate." + TEST_FILTER "InvalidImageUnitScenario.${privateCase}" + DISCOVERY_TIMEOUT 30 + PROPERTIES LABELS "${mglItestMagmaLabels}" TIMEOUT ${MGL_ITEST_TIMEOUT} + ${mglItestArmProperties} + ENVIRONMENT "${mglItestMagmaWireEnvironment}\;MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/p7-imageprivate${mglItestArmLogTag}-${privateCase}.log") + endforeach() + endif() endforeach() endif() diff --git a/MobileGL/MG_IntegrationTest/Scenarios/InvalidImageUnitScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/InvalidImageUnitScenario.cpp index 1723090b..afbcf133 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/InvalidImageUnitScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/InvalidImageUnitScenario.cpp @@ -28,6 +28,15 @@ // the store through the invalid unit changed no texel the invalid texture does have, and no GL // error was raised. // +// The two `AStoreThrough...` cases (codex closeout finding 2) store FIRST, through invalid unit A, +// then load through invalid unit B of the same shape and through A: a discarded store must never be +// loaded back. Magma's wire arm answered every invalid storage unit of one (format, target) from +// one shared placeholder image and failed both loads; it now binds a NULL storage descriptor +// (VK_EXT_robustness2 nullDescriptor), or, without one, a placeholder private to the (binding, +// unit) - forced on the host by MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1 (the +// `DirectVulkan.{Split,Spawn}.ImageUnitPrivate.` entries), where A's own load is the recorded +// CONTRACT-P7 §12 residual. +// // Arms. Split/spawn/tcp on both backends. Espryt forwards the unit to the driver, which applies // 8.26 itself. The MONOLITH DirectVulkan arm is skipped by name: it resolves the same binding to // "no descriptor" and drops the whole draw (VkTextureManager::GetOrCreateStorageImageView's @@ -41,6 +50,7 @@ #include "../Harness/HeadlessGL.h" #include "../Harness/ScenarioFixture.h" +#include "../Harness/SplitLane.h" #include "../Harness/SplitRuntimePeek.h" #ifdef GLAPI @@ -79,6 +89,30 @@ void main() { vec2 positions[3] = vec2[3](vec2(-1.0, -1.0), vec2(3.0, -1.0), vec2(-1.0, 3.0)); gl_Position = vec4(positions[gl_VertexID], 0.0, 1.0); } +)"; + + // Codex closeout finding 2: TWO INVALID UNITS MUST NOT ALIAS, AND AN INVALID STORE MUST NOT BE + // LOADED BACK. Magma's wire arm bound every invalid (and every empty) storage unit of one + // (format, target) to ONE shared placeholder image, cleared before the pass - so inside the + // pass a store through invalid unit A was there to be loaded through invalid unit B, and + // through A itself. 8.26: the store is discarded, both loads return zero. The store comes + // FIRST here (the four-unit shader above loads its invalid source before it stores to its + // invalid destination, so it could not see this), then an image barrier, then both loads, + // each into a complete destination that starts non-zero. + constexpr GLubyte kStoredThroughInvalid = 0x7F; + constexpr const char* kAliasComputeSource = R"(#version 430 core +layout(local_size_x = 8, local_size_y = 8) in; +layout(rgba8, binding = 0) writeonly uniform image2D u_loadedThroughB; +layout(rgba8, binding = 1) writeonly uniform image2D u_loadedThroughA; +layout(rgba8, binding = 2) coherent uniform image2D u_invalidA; +layout(rgba8, binding = 3) coherent readonly uniform image2D u_invalidB; +void main() { + ivec2 p = ivec2(gl_GlobalInvocationID.xy); + imageStore(u_invalidA, p, vec4(127.0 / 255.0)); + memoryBarrierImage(); + imageStore(u_loadedThroughB, p, imageLoad(u_invalidB, p)); + imageStore(u_loadedThroughA, p, imageLoad(u_invalidA, p)); +} )"; // The conformance case's fragment shader, including its trailing discard. @@ -289,9 +323,9 @@ void main() { return texels; } - GLuint BuildCompute() { + GLuint BuildCompute(const char* source = kComputeSource) { const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); - glShaderSource(shader, 1, &kComputeSource, nullptr); + glShaderSource(shader, 1, &source, nullptr); glCompileShader(shader); GLint ok = GL_FALSE; glGetShaderiv(shader, GL_COMPILE_STATUS, &ok); @@ -409,6 +443,99 @@ void main() { EXPECT_EQ(FirstGLError(), 0u) << "the readbacks raised a GL error (" << what << ")"; } + // Every texel of `texels` (one kEdge x kEdge RGBA8 level) loaded zero: R, G and B, which + // is what 8.26 fixes, and A as well where `strictAlpha` (the named-but-missing texel, + // whose conformance case compares all four; an EMPTY unit's placeholder on the arm + // without a null descriptor is the numeric-domain R32 one, which expands A to 1 - legal, + // 8.26 leaves A undefined). `storedAtOrigin` instead expects the stored 0x7F at (0, 0) + // in R, the one channel every placeholder format carries. + static void ExpectLoadedZero(const std::vector& texels, bool storedAtOrigin, bool strictAlpha, + const char* through, const char* what) { + int wrong = 0; + int firstX = -1, firstY = -1; + for (int y = 0; y < kEdge; ++y) { + for (int x = 0; x < kEdge; ++x) { + const std::size_t at = static_cast((y * kEdge + x) * 4); + const bool ok = storedAtOrigin && x == 0 && y == 0 + ? texels[at] == kStoredThroughInvalid + : texels[at] == 0 && texels[at + 1] == 0 && texels[at + 2] == 0 && + (!strictAlpha || texels[at + 3] == 0); + if (!ok) { + if (wrong++ == 0) { + firstX = x; + firstY = y; + } + } + } + } + if (wrong != 0) { + const std::size_t at = static_cast((firstY * kEdge + firstX) * 4); + ADD_FAILURE() << wrong << " of " << kEdge * kEdge << " texels loaded through " << through << " (" + << what << ") were not what 8.26 gives; first at (" << firstX << ", " << firstY + << ") = (" << int(texels[at]) << ", " << int(texels[at + 1]) << ", " + << int(texels[at + 2]) << ", " << int(texels[at + 3]) << ") (127 is the value " + << "stored through the invalid unit A, 17/34 means the pass never ran)"; + } + } + + // Units 2 (A) and 3 (B) are both invalid and of ONE shape - either both name the + // conformance case's missing mip level of a same-format texture, or both are empty. + // One dispatch stores 0x7F through A, barriers, and loads through B and through A. + void ExpectNoAliasThroughInvalidUnits(bool empty, const char* what) { + if (!LimitIsAtLeast(GL_MAX_COMPUTE_IMAGE_UNIFORMS, 4)) { + GTEST_SKIP() << "the compute stage has fewer than four image uniforms"; + } + // The Magma wire arm without a null storage descriptor (a device lacking + // VK_EXT_robustness2 nullDescriptor; forced on the host by the knob): each invalid + // unit gets a placeholder private to its (binding, unit). B's load is still zero - + // the units no longer alias - but A's load of A's own store in the same pass is + // not: that is the CONTRACT-P7 §12 residual, and here it is what PROVES the knob + // reached the server (the null-descriptor arm would read zero there too). + const bool privatePlaceholderArm = m_wire && Gl().BackendName() == "DirectVulkan" && + SplitLane::MarkerIsOne("MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER"); + const GLuint loadedThroughB = MakeComplete2D(kCompleteDestinationMagic); + const GLuint loadedThroughA = MakeComplete2D(kCompleteSourceMagic); + InvalidTexture invalidA{}; + InvalidTexture invalidB{}; + if (!empty) { + invalidA = MakeInvalid(Invalidity::MissingMipLevel, kInvalidDestinationMagic); + invalidB = MakeInvalid(Invalidity::MissingMipLevel, kInvalidSourceMagic); + } + ASSERT_EQ(FirstGLError(), 0u) << "building the textures raised a GL error (" << what << ")"; + + glBindImageTexture(0, loadedThroughB, 0, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + glBindImageTexture(1, loadedThroughA, 0, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + glBindImageTexture(2, invalidA.texture, invalidA.bindLevel, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + glBindImageTexture(3, invalidB.texture, invalidB.bindLevel, GL_FALSE, 0, GL_READ_WRITE, GL_RGBA8); + ASSERT_EQ(FirstGLError(), 0u) << "glBindImageTexture raised a GL error (" << what << ")"; + + m_program = BuildCompute(kAliasComputeSource); + ASSERT_NE(m_program, 0u); + glUseProgram(m_program); + glDispatchCompute(1, 1, 1); + glMemoryBarrier(GL_ALL_BARRIER_BITS); + EXPECT_EQ(FirstGLError(), 0u) << "the pass raised a GL error (" << what << ")"; + + ExpectLoadedZero(ReadLevel(GL_TEXTURE_2D, loadedThroughB, 0, 1), false, !empty, + "invalid unit B after a store through invalid unit A of the same shape", what); + ExpectLoadedZero(ReadLevel(GL_TEXTURE_2D, loadedThroughA, 0, 1), privatePlaceholderArm, !empty, + privatePlaceholderArm + ? "invalid unit A itself on the private-placeholder arm (0x7F at the origin " + "is the recorded same-unit residual AND the proof this arm ran; zero means " + "MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER did not reach the server)" + : "invalid unit A itself after its own store", + what); + if (!empty) { + // The discarded store changed nothing the texture has. + for (const InvalidTexture* invalid : {&invalidA, &invalidB}) { + EXPECT_EQ(ReadLevel(GL_TEXTURE_2D, invalid->texture, 0, 1), + Pattern(kEdge, kEdge, 1, invalid->magic)) + << "a store through an invalid unit modified the level its texture has (" << what << ")"; + } + } + EXPECT_EQ(FirstGLError(), 0u) << "the readbacks raised a GL error (" << what << ")"; + } + bool m_wire = false; std::vector m_textures; unsigned int m_program = 0; @@ -455,4 +582,18 @@ void main() { ExpectInvalidAccess(Invalidity::PastViewWindow, false, "past-view-window"); } + // Codex closeout finding 2: a store through invalid unit A, an image barrier, then loads + // through invalid unit B of the same (format, target) and through A - both zero. + TEST_F(InvalidImageUnitScenario, AStoreThroughAnInvalidUnitIsLoadedThroughNeitherAnotherInvalidUnitNorItself) { + if (!Ready() || IsSkipped()) return; + ExpectNoAliasThroughInvalidUnits(false, "missing-mip-level units"); + } + + // The same through two EMPTY units (glBindImageTexture(unit, 0, ...)): 8.26 makes those + // accesses invalid too, and Magma's wire arm answered them from the same shared placeholder. + TEST_F(InvalidImageUnitScenario, AStoreThroughAnEmptyUnitIsLoadedThroughNeitherAnotherEmptyUnitNorItself) { + if (!Ready() || IsSkipped()) return; + ExpectNoAliasThroughInvalidUnits(true, "empty units"); + } + } // namespace MGITest diff --git a/scripts/ci/spawn_lane_parity.py b/scripts/ci/spawn_lane_parity.py index 4a80209f..5700e13b 100644 --- a/scripts/ci/spawn_lane_parity.py +++ b/scripts/ci/spawn_lane_parity.py @@ -92,9 +92,13 @@ MAGMA_INPROC_ONLY = ("MagmaRunAheadScenario.", "MagmaWireCacheScenario.") # real probe's verdict, which the tcp copy cannot: same name on all three arms, no exception). # `.MsResolveElide.` (g5-msprobe critic) sets MGITEST_MAGMA_DEPTH_RESOLVE_PROBE=elide-subject, which # the same server-side arm choice reads to run the REAL probe with its render-pass resolve left out. +# Codex closeout finding 2 (cf-magma): `.ImageUnitPrivate.` sets +# MGITEST_MAGMA_FORCE_PRIVATE_IMAGE_PLACEHOLDER=1, which the SERVER's UniformManager reads to bind a +# (binding, unit)-private placeholder for an invalid image unit instead of a null descriptor - the +# knob-free `.ImageUnitWindow.` copies of the same two cases keep their tcp entries. MAGMA_SERVER_ENV_KNOB_NO_TCP = (".ShaderMip1.", ".ShaderMip2.", ".DepthMip.", ".DefaultBlitShape1.", ".MsResolve1.", ".StaleSerial.", ".MsFlip1.", - ".MsResolveBug.", ".MsFlipBug.", ".MsResolveElide.", + ".MsResolveBug.", ".MsFlipBug.", ".MsResolveElide.", ".ImageUnitPrivate.", ".Reclaim.MagmaWireReclaimScenario.RespecifiesWithNoDrawBetweenKeepTheLiveStoreCountBounded", ".Reclaim.MagmaWireReclaimScenario.RespecifyAndDrawEachStoreInOneFrameStaysWithinTheDeferredBudget", ".Reclaim.MagmaWireReclaimScenario.ManySmallRespecifyAndDrawRoundsStayUnderTheStoreCountCeiling",