mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-12 14:18:31 +09:00
[Fix] (MG_Impl, MG_Pipe, MG_IntegrationTest): close the four cross-package rows nobody claimed - symmetric vertex-input pairing asserts, the counter G10 reads, and the buffer ABA's Magma-only evidence
This commit is contained in:
@@ -995,36 +995,35 @@ namespace MobileGL::MG_Pipe {
|
|||||||
MGPipeSubsystemForDirty(MGPipeDirty::NewVertexAttribDefaults),
|
MGPipeSubsystemForDirty(MGPipeDirty::NewVertexAttribDefaults),
|
||||||
"set_vertex_attrib_defaults and NEW_VERTEX_ATTRIB_DEFAULTS must name one subsystem");
|
"set_vertex_attrib_defaults and NEW_VERTEX_ATTRIB_DEFAULTS must name one subsystem");
|
||||||
|
|
||||||
// P3a's pairing, in the two halves the contract commit can actually state.
|
// P3a's pairing, now stated as the SAME EQUALITY the four above are (contract-review
|
||||||
|
// m4, closed here).
|
||||||
//
|
//
|
||||||
// The four above compare the two maps directly, which is only possible once BOTH
|
// It was written with an escape hatch - `MGPipeSubsystemForDirty(...) == 0 ||` - because
|
||||||
// sides name the subsystem. MGPipeSubsystemForDirty is MG_Impl/Pipe/Tracker.h's and
|
// at the contract commit Tracker.h's bit 5 / 9 / 10 arms did not exist yet and the
|
||||||
// its bit 5 / 9 / 10 arms land with the client emitters, not here - so the direct
|
// direct form would have failed for a reason that was not a defect. That hatch was
|
||||||
// form would fail at this commit for a reason that is not a defect. What is stated
|
// explicitly conditional on the dirty half being unmapped, and the dirty half is now
|
||||||
// instead is exactly as strong in the direction that matters:
|
// mapped (Tracker.h:145-148), so it is removed: leaving it would mean a later edit that
|
||||||
|
// unmapped one of these bits again passed silently, which is precisely what these
|
||||||
|
// assertions exist to catch.
|
||||||
//
|
//
|
||||||
// (a) the emitter half names the vertex-input subsystem, so a later edit that moved
|
// AND ALL THREE COMPARE AGAINST SubsystemForEmitter, not against the constant. Two of
|
||||||
// it onto a different one fails here;
|
// them named kMGPipeSubsystemVertexInput directly, which asks a different and weaker
|
||||||
// (b) the two maps AGREE OR THE DIRTY HALF IS NOT MAPPED YET. The escape hatch is
|
// question: it pins the dirty half to a constant instead of pinning the two MAPS to
|
||||||
// the not-yet-mapped case only: the moment Tracker.h maps NEW_VERTEX_ELEMENTS
|
// each other, so an emitter row moved onto another subsystem would still satisfy them
|
||||||
// onto anything at all, this becomes the equality the four above are;
|
// while the emission gate and the residual-fill skip had begun to disagree. C.5's trap
|
||||||
// (c) and while the dirty half is unmapped the subsystem is NOT in
|
// is exactly that kind of near-miss. bind_vertex_elements is the family's only
|
||||||
// kMGPipeWiredSubsystems below, so no field can be skipped on the strength of a
|
// Coverage.def emitter row, so it is the emitter side of all three.
|
||||||
// call nobody emits. (c) is what makes (b)'s hatch safe rather than convenient.
|
|
||||||
static_assert(SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements) ==
|
static_assert(SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements) ==
|
||||||
kMGPipeSubsystemVertexInput,
|
kMGPipeSubsystemVertexInput,
|
||||||
"bind_vertex_elements must name the vertex-input subsystem");
|
"bind_vertex_elements must name the vertex-input subsystem");
|
||||||
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewVertexElements) == 0 ||
|
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewVertexElements) ==
|
||||||
MGPipeSubsystemForDirty(MGPipeDirty::NewVertexElements) ==
|
|
||||||
SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements),
|
SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements),
|
||||||
"bind_vertex_elements and NEW_VERTEX_ELEMENTS must name one subsystem");
|
"bind_vertex_elements and NEW_VERTEX_ELEMENTS must name one subsystem");
|
||||||
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewVertexBuffers) == 0 ||
|
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewVertexBuffers) ==
|
||||||
MGPipeSubsystemForDirty(MGPipeDirty::NewVertexBuffers) ==
|
SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements),
|
||||||
kMGPipeSubsystemVertexInput,
|
|
||||||
"set_vertex_buffers and NEW_VERTEX_BUFFERS must name one subsystem");
|
"set_vertex_buffers and NEW_VERTEX_BUFFERS must name one subsystem");
|
||||||
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewIndexBuffer) == 0 ||
|
static_assert(MGPipeSubsystemForDirty(MGPipeDirty::NewIndexBuffer) ==
|
||||||
MGPipeSubsystemForDirty(MGPipeDirty::NewIndexBuffer) ==
|
SubsystemForEmitter(MGPipeFieldEmitter::BindVertexElements),
|
||||||
kMGPipeSubsystemVertexInput,
|
|
||||||
"set_index_buffer and NEW_INDEX_BUFFER must name one subsystem");
|
"set_index_buffer and NEW_INDEX_BUFFER must name one subsystem");
|
||||||
// The two vertex views' capacity is one number on both sides of the boundary. This is
|
// The two vertex views' capacity is one number on both sides of the boundary. This is
|
||||||
// the one translation unit that sees the frontend constant and the MG_Pipe one, so it
|
// the one translation unit that sees the frontend constant and the MG_Pipe one, so it
|
||||||
|
|||||||
@@ -892,6 +892,24 @@ gtest_discover_tests(MobileGLIntegrationTest
|
|||||||
# table, and LookupVaoDrawMemo); it steers nothing on DirectGLES, and a lane that configured it
|
# table, and LookupVaoDrawMemo); it steers nothing on DirectGLES, and a lane that configured it
|
||||||
# there would be a permanent skip claiming to be a control.
|
# there would be a permanent skip claiming to be a control.
|
||||||
#
|
#
|
||||||
|
# SO THE BUFFER ABA's CORRUPTION EVIDENCE IS MAGMA-ONLY, AND THAT IS RECORDED RATHER THAN LEFT
|
||||||
|
# AS AN ABSENCE (gates m10, closed here as a statement rather than as a lane). P3a re-keys the
|
||||||
|
# BUFFER on DirectGLES - resource_* now dispatches by handle - so the obvious next move is a
|
||||||
|
# DirectGLES `.AbaControl` lane over ABufferAtARecycledAddressDoesNotInheritItsPredecessorsContents.
|
||||||
|
# It is not a registration-only change and it is therefore not made here: the knob has exactly
|
||||||
|
# one consumer in the tree (MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h's
|
||||||
|
# MagmaPipeAbaControlDefeatsIdentity, which is what the MGL_ITEST_MAGMA_ABA probe above looks
|
||||||
|
# for), so a DirectGLES lane would run with the knob inert, fail to reproduce the corruption it
|
||||||
|
# asserts, and go RED in an always-on integration-gpu lane - the exact failure mode the header
|
||||||
|
# of HandleRecycleScenario.cpp records this file already having had once.
|
||||||
|
#
|
||||||
|
# What that costs, stated so the next reader does not have to re-derive it: on DirectGLES the
|
||||||
|
# buffer case's Handles arm proves the re-key does NOT alias, and nothing proves the reproducer
|
||||||
|
# could still see an aliasing that was reintroduced there. The positive control for that is a
|
||||||
|
# Features.PipeHandleAbaControl consumer over Espryt's resource slot table - one `if` in
|
||||||
|
# GetOrCreate / FindByHandle, the way MagmaPipeClaimSlotMemos is Magma's - and it belongs with
|
||||||
|
# whoever next touches that table, not with a lane registration.
|
||||||
|
#
|
||||||
# It gets TWO lanes, because there are two arms and the control has to reach the one P2 SHIPS.
|
# It gets TWO lanes, because there are two arms and the control has to reach the one P2 SHIPS.
|
||||||
# `AbaControl` is D18's lane verbatim (MOBILEGL_PIPE_PUSH=0, the pre-handle arm) and defeats the
|
# `AbaControl` is D18's lane verbatim (MOBILEGL_PIPE_PUSH=0, the pre-handle arm) and defeats the
|
||||||
# lifetime-id/address guards; `AbaControlHandles` runs the handle arm (MOBILEGL_PIPE_LEGACY_MEMOS=0,
|
# lifetime-id/address guards; `AbaControlHandles` runs the handle arm (MOBILEGL_PIPE_LEGACY_MEMOS=0,
|
||||||
|
|||||||
@@ -628,6 +628,20 @@ namespace MobileGL::MG_Pipe {
|
|||||||
g_applier.VertexBufferCount = 0;
|
g_applier.VertexBufferCount = 0;
|
||||||
g_applier.VertexFetchBaseInstance = 0;
|
g_applier.VertexFetchBaseInstance = 0;
|
||||||
g_applier.IndexBuffer = MGPIndexBuffer{};
|
g_applier.IndexBuffer = MGPIndexBuffer{};
|
||||||
|
// m6 / wire n6, written down rather than left to be rediscovered: THIS counter is
|
||||||
|
// per-applier and is zeroed at every make-current, while MG_Util::PipeStats' `mpr` -
|
||||||
|
// emitted from the CLIENT at MG_Impl/Pipe/PipeFill.cpp's MGPipeEmitMapPersistent - is
|
||||||
|
// process-wide and is windowed by EndFrame. The two therefore disagree across a context
|
||||||
|
// switch, by design and not by accident: this one answers "how many round trips has THIS
|
||||||
|
// applier been asked for since it was last reset", which is what a unit case driving the
|
||||||
|
// applier directly wants, and PipeStats' answers "how many did the process take in this
|
||||||
|
// window", which is what a lane reading a log line wants.
|
||||||
|
//
|
||||||
|
// WHICH ONE THE GATES ASSERT ON, because that was the open question: G10
|
||||||
|
// (StorageBufferRegrow) and G12 read PipeStats' `mpr` out of the lane's own log through
|
||||||
|
// MG_IntegrationTest/Harness/PipeStatsWindow.h - they cannot link this symbol at all, on
|
||||||
|
// Android or anywhere else - so a make-current inside a scenario cannot silently reset
|
||||||
|
// what they measure. Nothing outside MG_Test reads the member below.
|
||||||
g_applier.MapPersistentRoundtrips = 0;
|
g_applier.MapPersistentRoundtrips = 0;
|
||||||
// THE TWO SERIALS ADVANCE; THEY ARE NOT ZEROED. They are MGGens, and an MGGen that
|
// THE TWO SERIALS ADVANCE; THEY ARE NOT ZEROED. They are MGGens, and an MGGen that
|
||||||
// walks backwards is not one. There are exactly three things a reset can do to a
|
// walks backwards is not one. There are exactly three things a reset can do to a
|
||||||
|
|||||||
Reference in New Issue
Block a user