From e00db8e64f1f978b5070ce1aa4a50736338b9604 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 8 Oct 2026 15:53:42 -0400 Subject: [PATCH] Pipe: every verb names its record op in a verb-indexed table (static_assert: one row per verb), so a verb syncs only when its record does; the barriered fallback that quiesced the applier once per frame at the blit is gone (FCL inproc GL-thread wait 1.65 ms/frame; red-once: blit row removed aborts the run-ahead blit test, missing row fails to compile). --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 84 ++-------- MobileGL/MG_Pipe/VerbRecordOps.h | 163 +++++++++++++++++++ MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp | 28 ++++ MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 19 +++ 4 files changed, 227 insertions(+), 67 deletions(-) create mode 100644 MobileGL/MG_Pipe/VerbRecordOps.h diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 4975b25e..a1433b4a 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #if MOBILEGL_PIPE_VERIFY #include @@ -375,85 +376,34 @@ namespace MobileGL::MG_Pipe { return session != nullptr && session->RunAheadArmed(); } - // THE WIRE OP A VERB BOUNDARY BECOMES, which is the join MGPipeVerbForWireOp draws in - // the other direction. Built by walking the op space once at compile time rather than - // written out, because a second hand-written table is a second thing to forget a row - // in - and the generator already refuses a verb-shaped op with no row. - constexpr MGPWireOp WireOpForVerb(MGPipeVerb verb) { - for (SizeT i = 0; i < static_cast(MGPWireOp::kOpCount); ++i) { - const auto op = static_cast(i); - if (MGPipeVerbForWireOp(op) == verb) return op; - } - return MGPWireOp::kOpCount; - } - // CONTRACT-P5E §2.1's predicate, ASKED AT THE VALIDATE POINT - which is before the // record exists, so it is asked of the verb and the live context rather than of the // payload. The two halves must agree with MGPipeBarriered(op, payload, applierState), // which is what the server computes, so each clause is the same clause: // - // 1. the static WaitClass column - the same generated table, same op; + // 1. the static WaitClass column - of the op the verb's emitter publishes, + // named by MGP_VERB_RECORD_OP_LIST (MG_Pipe/VerbRecordOps.h); // 2. kCtxVerb inside an open XFB span - ctx.IsTransformFeedbackActive() here, // the applied MGPContextValues mirror there, and set_context_values precedes the // verb on the ring, so the two read the same value (§2.1); // 3. a draw carrying kDrawClientArrays - NOT asked here, because under run-ahead // such a draw never reaches a record at all: vi refuses it on this very thread in - // EmitDrawRecord's array arm (§5.1, ruling 2). Asking it here would be a second - // spelling of vi's "does any enabled attribute lack a buffer" walk, and two - // spellings of an escalation is precisely what ruling 3 replaced. + // EmitDrawRecord's array arm (§5.1, ruling 2). // - // AN OP WITH NO VERB ROW ANSWERS BARRIERED. A verb the join does not know is one this - // file cannot reason about, and the safe answer - fill it, wait for it - is also the - // pre-P5e answer. - // `ctx` MAY BE NULL, and that is not a convenience: the validate point asks this - // question BEFORE it has decided whether there is anything to fill, so that the answer - // is about the RECORD and not about the state of the block. A null context has no open - // transform-feedback span, so clause 2 is false for it - and the verb is a no-op below - // either way. + // P15: THERE IS NO FALLBACK. The answer used to come from inverting MGP_VERB_OP_LIST + // (op -> verb, many-to-one), and a verb the inverse missed answered BARRIERED - which + // the draw family and then the blit family each paid for until a special case named + // them (the blit: one applier quiesce per frame on FCL). The verb -> record table has a + // row for every verb by static_assert, so async is the default and a verb syncs only + // because the record it publishes says so. + // + // `ctx` MAY BE NULL: the validate point asks this question BEFORE it has decided + // whether there is anything to fill. A null context has no open transform-feedback + // span, so clause 2 is false for it - and the verb is a no-op below either way. Bool ClientVerbIsBarriered(MGPipeVerb verb, GLContext* ctx) { - MGPWireOp op = WireOpForVerb(verb); - // ---- P5e (ra2): THE JOIN IS MANY-TO-ONE ON THE DRAW FAMILY, SO THE INVERSE IS NOT - // A FUNCTION, AND THE DEFAULT BELOW WAS ANSWERING FOR NINETEEN VERBS ----------------- - // - // MGP_VERB_OP_LIST carries ONE row for the whole draw family - `DrawVbo -> - // DrawArrays` - because that is the direction the SERVER needs: a draw_vbo record - // stamps a verb boundary and DrawArrays is the name it prints. Inverting it - // verb-first therefore answers kOpCount for DrawElements, DrawElementsBaseVertex, - // DrawElementsIndirect, MultiDrawElementsBaseVertex and every other indexed / - // instanced / multi / indirect verb - all of which emit exactly that same draw_vbo. - // - // The conservative default below reads "a verb the join does not know answers - // BARRIERED: fill it, wait for it". THE FILL HAPPENS AND THE WAIT DOES NOT. The wait - // is not decided here - it is decided per RECORD, by MGPipeBarriered(op, payload, - // st) at the publish (§2.1) - and the record is a draw_vbo, whose wait class is - // kWaitNone. So every indexed draw filled gPipeInputs while telling - // RefusePipeInputsTouchWhileApplierOwnsIt, through isBarrieredFill, that this thread - // was about to park behind it, and then ran on without parking. The apply thread was - // measured inside a record at that instant, and the abort it produced named the - // CLIENT's verb - a verb the applier cannot stamp, which is what identifies the - // writer (report §2). - // - // So the family's one row is applied to the family. Every other unknown verb keeps - // the conservative answer, which is now honest rather than hoped for: the fill site - // establishes quiescence before it writes (MGPipeValidateForVerb below), so a - // barriered fill no longer rests on a park that may never come. - static_assert(MGPipeVerbForWireOp(MGPWireOp::DrawVbo) == MGPipeVerb::DrawArrays, - "the draw family's representative row moved; the fallback below names " - "draw_vbo because that is the record every kDraw verb emits"); - // P5e (ra2): THE CLIENT'S HALF OF ESCALATION (iii) WAS HERE AND WENT WITH IT - // (ID-133, withdrawn by ID-136). It named MultiDrawArrays / MultiDrawElements / - // MultiDrawElementsBaseVertex so the client would FILL for the records the server - // was escalating - a fill the server then read as a barriered pull. Retiring that - // pull outright (MultiDraw.cpp's BoundDrawIndirectBufferId takes a handle arm) left - // nothing for the fill to serve, so a plain multi-draw is an ordinary kDraw verb - // again and takes the draw_vbo answer below. The pair is the phase's own lesson - // written twice: a wait added to make a lane green is paid by an arm, and here the - // arm was the default tier the phone ships. - if (op == MGPWireOp::kOpCount && - kMGPipeVerbClass[static_cast(verb)] == MGPipeVerbClass::kDraw) { - op = MGPWireOp::DrawVbo; - } - if (op == MGPWireOp::kOpCount) return true; + const MGPWireOp op = MGPipeRecordOpForVerb(verb); + // No record (answered from the caps mirror): no apply can read a fill for it. + if (op == kMGPipeNoRecord) return false; if (MGPipeWaitClassFor(op) != kWaitNone) return true; if (MGPipeCallClassFor(op) == kCtxVerb && ctx != nullptr && ctx->IsTransformFeedbackActive()) { diff --git a/MobileGL/MG_Pipe/VerbRecordOps.h b/MobileGL/MG_Pipe/VerbRecordOps.h new file mode 100644 index 00000000..b89da5c6 --- /dev/null +++ b/MobileGL/MG_Pipe/VerbRecordOps.h @@ -0,0 +1,163 @@ +// MobileGL - MobileGL/MG_Pipe/VerbRecordOps.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#pragma once + +#include + +#include + +// THE VERB -> RECORD TABLE (P15). Every MGPipeVerb names the record op its emitter publishes, and +// the client's barrier decision at the validate point is that op's wait class (PipeCalls.def) and +// nothing else. Commands are recorded and replayed asynchronously by default; a verb syncs only +// because the record it emits has a wait class that says so. +// +// IT IS THE FORWARD DIRECTION. MGP_VERB_OP_LIST (FieldOwnership.def) is op -> verb, the +// direction the SERVER needs to stamp a verb boundary, and it is many-to-one (one draw_vbo row +// for twenty draw verbs, one blit row for two blit verbs). Inverting it verb-first answered +// "no row" for every verb but the representative, and the old fallback turned "no row" into +// "barriered" - a forced applier quiesce per verb (FCL: one per frame from the blit, 1.65 ms of +// GL-thread wait). Here there is no fallback in either direction: a verb with no row, or with two, +// is a compile error (the static_assert below). +// +// A verb whose emitter can publish more than one op names the one with the STRONGEST wait class +// (the fill decision is made before the record exists, so it must cover the worst record the verb +// may emit). kMGPipeNoRecord names a verb that publishes nothing: the client answers it from the +// caps mirror, so no apply can read a fill for it. +// +// X(Verb, Op) +// clang-format off +#define MGP_VERB_RECORD_OP_LIST(X) \ + /* The draw family: every draw verb is one draw_vbo record. */ \ + X(DrawArrays, DrawVbo) \ + X(DrawElements, DrawVbo) \ + X(DrawElementsBaseVertex, DrawVbo) \ + X(MultiDrawArrays, DrawVbo) \ + X(MultiDrawElements, DrawVbo) \ + X(MultiDrawElementsBaseVertex, DrawVbo) \ + X(MultiDrawElementsIndirect, DrawVbo) \ + X(MultiDrawArraysIndirect, DrawVbo) \ + X(MultiDrawElementsIndirectCount, DrawVbo) \ + X(MultiDrawArraysIndirectCount, DrawVbo) \ + X(DrawRangeElementsBaseVertex, DrawVbo) \ + X(DrawRangeElements, DrawVbo) \ + X(DrawElementsInstancedBaseVertexBaseInstance, DrawVbo) \ + X(DrawElementsInstancedBaseVertex, DrawVbo) \ + X(DrawElementsInstancedBaseInstance, DrawVbo) \ + X(DrawElementsInstanced, DrawVbo) \ + X(DrawArraysInstancedBaseInstance, DrawVbo) \ + X(DrawArraysInstanced, DrawVbo) \ + X(DrawElementsIndirect, DrawVbo) \ + X(DrawArraysIndirect, DrawVbo) \ + /* Every clear entry point is one clear record (EmitClear / EmitF1Clear). */ \ + X(Clear, Clear) \ + X(ClearBufferfi, Clear) \ + X(ClearBufferfv, Clear) \ + X(ClearBufferuiv, Clear) \ + X(ClearBufferiv, Clear) \ + X(ClearNamedFramebufferfv, Clear) \ + X(ClearNamedFramebufferfi, Clear) \ + X(ClearNamedFramebufferiv, Clear) \ + X(ClearNamedFramebufferuiv, Clear) \ + X(BlitFramebuffer, Blit) \ + X(BlitNamedFramebuffer, Blit) \ + X(CopyTexImage2D, CopyFramebufferToTexture) \ + X(CopyTexSubImage2D, CopyFramebufferToTexture) \ + X(CopyImageSubData, ResourceCopyRegion) \ + X(GenerateMipmap, GenerateMipmap) \ + /* read_pixels (reply) or read_pixels_to_buffer (pack buffer): the reply wins. */ \ + X(ReadPixels, ReadPixels) \ + /* get_texture_image (reply) or its pack-buffer half: the reply wins. */ \ + X(GetTexImage, GetTextureImage) \ + X(GetTextureImage, GetTextureImage) \ + X(DispatchCompute, LaunchGrid) \ + X(DispatchComputeIndirect, LaunchGrid) \ + X(MemoryBarrier, MemoryBarrier) \ + X(MemoryBarrierByRegion, MemoryBarrier) \ + X(BindImageTexture, BindShaderImage) \ + X(GetIntegeri_v, kMGPipeNoRecord) \ + X(ShaderStorageBlockBinding, SetStorageBlockBinding) \ + X(FenceSync, FenceCreate) \ + X(ClientWaitSync, FenceWait) \ + X(WaitSync, FenceWaitServer) \ + X(DeleteSync, FenceDestroy) \ + X(GetSyncStatus, FenceStatus) \ + X(IsTimerQuerySupported, kMGPipeNoRecord) \ + /* Query begins publish query_create then query_begin / query_counter. */ \ + X(BeginTimeElapsedQuery, QueryBegin) \ + X(EndTimeElapsedQuery, QueryEnd) \ + X(QueryCounterTimestamp, QueryCounter) \ + X(IsQueryResultAvailable, QueryAvailable) \ + X(GetQueryResult64, QueryResult) \ + X(DeleteBackendQuery, QueryDestroy) \ + X(BeginOcclusionQuery, QueryBegin) \ + X(EndOcclusionQuery, QueryEnd) \ + X(BeginXfbPrimitivesQuery, QueryBegin) \ + X(EndXfbPrimitivesQuery, QueryEnd) \ + X(PatchParameteri, PatchParameter) \ + X(BeginTransformFeedback, BeginStreamOutput) \ + X(EndTransformFeedback, EndStreamOutput) \ + X(PauseTransformFeedback, PauseStreamOutput) \ + X(ResumeTransformFeedback, ResumeStreamOutput) \ + X(BindTransformFeedback, BindStreamOutput) \ + /* delete_stream_output, preceded by a bind_stream_output when it was bound. */ \ + X(DeleteTransformFeedback, BindStreamOutput) \ + X(GetGpuTimestampNs, QueryTimestamp) +// clang-format on + +namespace MobileGL::MG_Pipe { + // The op a verb names when it publishes no record at all. + inline constexpr MGPWireOp kMGPipeNoRecord = MGPWireOp::kOpCount; + + struct MGPipeVerbRecordOpRow { + MGPipeVerb Verb; + MGPWireOp Op; + }; + + namespace VerbRecordOpsDetail { + constexpr auto MakeRows() { + using enum MGPWireOp; + return std::array{ +#define MGP_VERB_RECORD_OP_ROW(Verb, Op) MGPipeVerbRecordOpRow{MGPipeVerb::Verb, Op}, + MGP_VERB_RECORD_OP_LIST(MGP_VERB_RECORD_OP_ROW) +#undef MGP_VERB_RECORD_OP_ROW + }; + } + inline constexpr auto kRows = MakeRows(); + + // Every verb exactly once: a verb with no row (or two) is a compile error, never a + // runtime default in either direction. + constexpr Bool EveryVerbHasExactlyOneRow() { + for (SizeT verb = 0; verb < static_cast(MGPipeVerb::kVerbCount); ++verb) { + SizeT rows = 0; + for (const auto& row : kRows) { + if (static_cast(row.Verb) == verb) ++rows; + } + if (rows != 1) return false; + } + return true; + } + static_assert(kRows.size() == static_cast(MGPipeVerb::kVerbCount), + "MGP_VERB_RECORD_OP_LIST must have exactly one row per MGPipeVerb"); + static_assert(EveryVerbHasExactlyOneRow(), + "an MGPipeVerb has no row (or two) in MGP_VERB_RECORD_OP_LIST: name the record op " + "its emitter publishes"); + + constexpr auto MakeTable() { + Array(MGPipeVerb::kVerbCount)> table{}; + for (const auto& row : kRows) table[static_cast(row.Verb)] = row.Op; + return table; + } + inline constexpr auto kTable = MakeTable(); + } // namespace VerbRecordOpsDetail + + // The record op `verb` publishes (kMGPipeNoRecord: none). + constexpr MGPWireOp MGPipeRecordOpForVerb(MGPipeVerb verb) { + return VerbRecordOpsDetail::kTable[static_cast(verb)]; + } +} // namespace MobileGL::MG_Pipe diff --git a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp index 6a6802a8..66b45f1b 100644 --- a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp +++ b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp @@ -38,6 +38,7 @@ #include #include "Includes.h" +#include #include #include @@ -252,6 +253,33 @@ TEST_F(FieldOwnershipTest, TheSevenStickyForwardsAgreeWithTheirFieldRows) { } } +// P15: the verb -> record table (MG_Pipe/VerbRecordOps.h) is the client's whole barrier +// answer, so every row has to agree with what the server will do with the record: the server +// stamps the op as MGPipeVerbForWireOp(op), and that verb must be of the SAME fill class as the +// verb that published it - otherwise an unbarriered record would be applied under another class's +// field mask. A row whose op is not a stamp point at all would apply under the previous verb's +// serial. (Completeness - one row per verb - is a static_assert in the header.) +TEST_F(FieldOwnershipTest, EveryVerbsRecordOpIsStampedAsAVerbOfTheSameFillClass) { + Uint32 noRecord = 0; + for (SizeT i = 0; i < static_cast(MGPipeVerb::kVerbCount); ++i) { + const auto verb = static_cast(i); + const MGPWireOp op = MGPipeRecordOpForVerb(verb); + if (op == kMGPipeNoRecord) { + ++noRecord; + continue; + } + const MGPipeVerb stamped = MGPipeVerbForWireOp(op); + ASSERT_NE(stamped, MGPipeVerb::kVerbCount) << kMGPipeVerbNames[i] << ": its record op stamps no verb"; + EXPECT_EQ(kMGPipeVerbClass[static_cast(stamped)], kMGPipeVerbClass[i]) + << kMGPipeVerbNames[i] << " publishes a record stamped as " << kMGPipeVerbNames[static_cast(stamped)]; + } + // GetIntegeri_v and IsTimerQuerySupported, answered from the caps mirror. + EXPECT_EQ(noRecord, 2u); + EXPECT_EQ(MGPipeRecordOpForVerb(MGPipeVerb::BlitNamedFramebuffer), MGPWireOp::Blit); + EXPECT_EQ(MGPipeRecordOpForVerb(MGPipeVerb::DrawElements), MGPWireOp::DrawVbo); + EXPECT_EQ(MGPipeRecordOpForVerb(MGPipeVerb::ClearBufferfv), MGPWireOp::Clear); +} + // Pin every boundary's representative verb, the complete count, and the three // verb-shaped calls that are exempt by name. TEST_F(FieldOwnershipTest, VerbBoundaryOpsCoverEveryVerbShapedCall) { diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index a1e353c8..702b96e8 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -2828,6 +2828,25 @@ TEST(RemoteRunAhead, AnIndexedDrawVerbIsUnbarrieredAndTouchesPipeInputsNotAtAll) ExpectChildSuccess(child); } +// P15: the blit family. Every blit reaches the validate point as BlitNamedFramebuffer and +// publishes one `blit` record (kWaitNone, stamped as BlitFramebuffer), so it is unbarriered: no +// fill and no applier quiesce. A barriered answer here was a forced wait once per frame on FCL +// (MC blits its main target to the window every frame). +// +// THE RED (done once while writing it): take the blit rows out of MGP_VERB_RECORD_OP_LIST's +// answer and this case aborts with Fatal{RoleViolation, "gPipeInputs"}. +TEST(RemoteRunAhead, ABlitNamedVerbIsUnbarrieredAndTouchesPipeInputsNotAtAll) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::NoteApplyThreadEnteredApplier(); + MGPipeValidateForVerb(MGPipeVerb::BlitNamedFramebuffer); + ClientSession::NoteApplyThreadLeftApplier(); + MGPipeLeaveVerb(); + ClientSessionInstance().Stop(); + }); + ExpectChildSuccess(child); +} + // §3.3's RED-ONCE, and the one that turns the strict lane into a gate. The probe is the // sticky forward itself, run on the apply thread inside a server-stamped verb with the // current record marked UNBARRIERED - which is the state the sink puts that thread in for