From 985ae62dfa1d0c671d6879df91eae1cae0db0f48 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 12:58:58 -0400 Subject: [PATCH 1/4] [MG_Backend, MG_Pipe] (Disaggregated): P5e - throw both run-ahead constants (NOT FOR MERGE YET) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit kMGPipeP5eRunAheadReady (MG_Backend/Init.cpp) and kMGPipeP5eClientWaitRuleLanded (MG_Pipe/PipeApply.h) both to true. They are ONE switch with two spellings and the integration commit throws both. THIS BRANCH IS NOT MERGEABLE AS IT STANDS and is parked so the defect it exposes can be worked on against a reproducible head. Measured on this commit: build rc=0 · unit 2254/2254 · gens rc=0 · flavours rc=0 (pull 915 TUs, push 929 TUs) integration-split 112 / 181 (69 failed) integration-gpu 1290 / 1359 (69 failed) and the failures are FLAKY: 71 failed at -j 4, 68 at -j 1, and a single entry run alone passes and then aborts on a repeat. A census over all 181 entries under each entry's own command and ENVIRONMENT gives 70 red on 17 distinct @ pairs, over just FOUR fields: 27 GetRenderStateParametersVersion 25 GetTextureContextId 16 GetBufferBindingSlot 2 IsCapabilityEnabled spread across eleven verbs - Clear, DrawArrays, DrawElements, DrawElementsBaseVertex, DrawElementsIndirect, DrawElementsInstancedBaseVertex, EndTransformFeedback, GetIntegeri_v, GetSyncStatus, MultiDrawElementsBaseVertex, ReadPixels. Several of those verbs are BARRIERED, which is what makes this interesting: 16 of the 70 abort on the UNBARRIERED arm (why=UNBARRIERED) and the other 54 are the plain unfresh-read abort with no [BARRIER-PULLED] bracket at all, i.e. a RECORD-SUPPLIED field that the freshness stamp says was never filled. One field at one verb, consistently, would be a missing migration. Four fields scattered over eleven verbs, flaky run to run, including barriered ones, is a RACE - and the obvious candidate is that gPipeInputs is shared mutable state whose mutual exclusion WAS the lockstep. The client still fills it for barriered verbs while the apply thread is draining earlier unbarriered records, so the client's fill and the server's verb-boundary stamp can now overlap in time. That is consistent with P5d's own research conclusion, recorded in the roadmap, that retiring this barrier needs gPipeInputs VERSIONING or double-buffering, deferred to P11 after P3b/P4b's handle-keyed twin tables. The strict lane being hard green (181/181, zero Fatal) is necessary and NOT sufficient, and this commit is the proof: strict runs under lockstep, where ApplyOne stamps every record barriered, so it cannot see anything that only happens when the client stops waiting. The lane measures remaining DEBT; it does not measure whether run-ahead is correct. --- MobileGL/MG_Backend/Init.cpp | 2 +- MobileGL/MG_Pipe/PipeApply.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Backend/Init.cpp b/MobileGL/MG_Backend/Init.cpp index 341e7ece..6d947032 100644 --- a/MobileGL/MG_Backend/Init.cpp +++ b/MobileGL/MG_Backend/Init.cpp @@ -95,7 +95,7 @@ namespace MobileGL::MG_Backend { // server into a run-ahead one, because the failure mode is not a slow frame - it is // the apply thread reading client memory that has already moved, which renders wrong // rather than aborting. - constexpr Bool kMGPipeP5eRunAheadReady = false; + constexpr Bool kMGPipeP5eRunAheadReady = true; // P5e (sb, ID-106). DirectGLES GAINS BIT 13 - the indexed buffer binding points - and // Magma deliberately does not. c0e landed the phase constant and the dirty-bit map with diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index a791c97b..502a9a44 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -992,7 +992,7 @@ namespace MobileGL::MG_Pipe { // Computing the predicate anyway (PipeApplier::ApplyOne) is deliberate: it keeps the // function exercised on every record for the whole phase rather than first run on the day // it starts deciding. - inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = false; + inline constexpr Bool kMGPipeP5eClientWaitRuleLanded = true; // P5c (rv): the two serials the PipeInputs texture-shutter accessors answer with under a // server-stamped verb (FieldOwnership.def, APPLIER_DERIVED). Free functions rather than From 2d03565f8727d520d4fc35b1b1eb4a74db386f32 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 13:38:40 -0400 Subject: [PATCH 2/4] [MG_Impl, MG_Remote, MG_Test] (Disaggregated): P5e ra2 - a barriered fill must be quiescent, not merely about to park MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ID-132's finding, diagnosed: with the flip thrown the 70 red lane entries are ONE race and two debts. The race is the client's residual fill writing gPipeInputs while the apply thread is inside an earlier UNBARRIERED record. Measured interleaving, verbatim: `[MobileGLIntegra/ERROR] RA2-PROBE fill verb=DrawElements wireop=81 waitclass=-1 applierInside=1` then `[mgl-srv-apply/ERROR] RA2-PROBE unfresh field=IsCapabilityEnabled verb=DrawElements serverStamped=0 ownership=RECORD-SUPPLIED` then `[mgl-srv-apply/FATAL] MGPipe: Fatal{UnmigratedPipeInput, "IsCapabilityEnabled@DrawElements"}`. The GL thread's MGPipeServerClearVerbBoundary withdrew the applier's own stamp mid-record. - The marker's verb is the CLIENT's verb, which is what identifies the writer without a probe at all: the applier can only ever put a verb of MGP_VERB_OP_LIST into the block, and 40 of the 54 unbracketed aborts named a verb that is not in it. - A/B control on the same binary and the same records: MOBILEGL_IPC_RUN_AHEAD=0 is 8/8 green where the default arm is 2/6 red, so the defect is in the wait rule and not in a record. - RefusePipeInputsTouchWhileApplierOwnsIt could not fire on any of it: its run-ahead arm read `if (isBarrieredFill) return;`, and that sentence is a claim about the FUTURE ("this thread is about to park behind the record"). The order at the validate point is fill, then emit, then park. So the arm now tests the fact - `isBarrieredFill && !ApplyThreadIsInsideApplier()` - and the fill sites MAKE it true first with §2.5's forced wait (QuiesceApplierBeforeFill). - TWICE PER VERB, because the block is written in two phases that straddle publication: the serial bump / stamp withdrawal / verb rename run before the fourteen emitters and the 63-field walk of step 4 runs after them, and the records those emitters published are records whose apply reads the block. - ClientVerbIsBarriered: MGP_VERB_OP_LIST joins the whole draw family through one row (DrawVbo -> DrawArrays), so inverting it verb-first answered kOpCount for DrawElements and eighteen other draw verbs, and the "unknown verb answers barriered" default filled for each while the record it published was a kWaitNone draw_vbo it never parked behind. The family's row is applied to the family; every other unknown verb keeps the conservative answer, which the wait above now makes honest. Measured alone it is worth 112 -> 133 isplit; with the wait in place the lane is 161 either way, so this half is the PERFORMANCE half - without it every indexed draw takes a forced drain and run-ahead is lockstep by another name on the draw path. - Red-once A: put the guard's arm back to `if (isBarrieredFill) return;` and RemoteGuards.ClientBarrieredFillWhileTheApplierIsInsideUnderRunAheadIsFatalByName exits 0 instead of aborting (`Value of: DiedOfAbort(child) Actual: false`), while its green control keeps passing. Red-once B: delete the kDraw fallback and RemoteRunAhead.AnIndexedDrawVerbIsUnbarrieredAndTouchesPipeInputsNotAtAll dies of signal 6 with `Fatal{RoleViolation, "gPipeInputs"} - the GL thread touched gPipeInputs (MGPipeValidateForVerb) on a RUN-AHEAD session in a barriered fill that did not first wait for the applier to catch up`. - Both new cases run in the ordinary unit lane with run-ahead armed by the fixture, which is what ID-132(1) asks for: the strict lane runs under lockstep and cannot see this class. - Gate on the flipped head: unit 2256/2256, isplit 161/181 (was 112), gpu 1339/1359 (was 1290), gens rc=0, flavours rc=0. The remaining 20 are two named debts, not this race: 18 are `GetBufferBindingSlot@DrawArrays [UNBARRIERED]` on the two indirect multi-draw tiers, whose retiring phase is P8, and 2 are ID-82's own refusal of client vertex arrays. - The arm is stated and not inferred (ID-81): WaitForApplyToCatchUp's first test is `m_runAheadArmed && m_started`, which reduces to Transport != Monolith. Under lockstep, under monolith and under MOBILEGL_IPC_RUN_AHEAD=0 every line added here is a call that returns, so those arms are unchanged. --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 101 +++++++++++++++++++- MobileGL/MG_Remote/Client/ClientSession.cpp | 38 ++++++-- MobileGL/MG_Remote/Client/ClientSession.h | 13 ++- MobileGL/MG_Test/Wire/RemoteClientTest.cpp | 67 +++++++++++++ 4 files changed, 209 insertions(+), 10 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 8e05a588..0c851f34 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -394,7 +394,39 @@ namespace MobileGL::MG_Pipe { // 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) { - const MGPWireOp op = WireOpForVerb(verb); + 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"); + if (op == MGPWireOp::kOpCount && + kMGPipeVerbClass[static_cast(verb)] == MGPipeVerbClass::kDraw) { + op = MGPWireOp::DrawVbo; + } if (op == MGPWireOp::kOpCount) return true; if (MGPipeWaitClassFor(op) != kWaitNone) return true; if (MGPipeCallClassFor(op) == kCtxVerb && ctx != nullptr && @@ -404,6 +436,40 @@ namespace MobileGL::MG_Pipe { return false; } + // ---- P5e (ra2), CONTRACT-P5E §3.5 AMENDED: A BARRIERED FILL MAKES ITSELF QUIESCENT ---- + // + // §3.5 exempted the residual fill of a BARRIERED record from the single-writer rule on + // the ground that "this thread is about to park behind it". That is a claim about the + // FUTURE, and the write is in the PRESENT: the order at the validate point is fill, + // then emit, then park, and between the fill and the park the apply thread is still + // draining the UNBARRIERED records the client ran ahead of. The claim was therefore + // never an argument about this instant, and the guard that took it - the + // `if (isBarrieredFill) return;` arm of RefusePipeInputsTouchWhileApplierOwnsIt - could + // not fire however wrong the fill was. + // + // This makes the claim TRUE instead of asserting it. §2.5's forced wait is exactly + // "publish nothing, wait for the applier to reach LastPublishedSeq, drain the reverse + // channel", which is the definition of the window the fill needs, and it already exists + // for glFinish and for BackendObject_Remote's forwarders. + // + // IT IS CALLED AT EVERY GL-THREAD WRITE INTO THE BLOCK, not once per verb, because the + // validate point writes the block in TWO phases that straddle record publication: the + // serial bump / stamp withdrawal / verb rename run BEFORE the emitters, and the 63-field + // walk of step 4 runs AFTER them - and the records the emitters published are records + // whose apply READS the block. One wait cannot cover both halves. + // + // THE ARM IS STATED, NOT INFERRED (ID-81): WaitForApplyToCatchUp's own first test is + // `m_runAheadArmed && m_started`, and m_runAheadArmed is the latch of + // `Transport != Monolith && Ipc.RunAhead && kMGPipeP5eRunAheadReady && the caps bit`. So + // on the monolith arm, on the pull build and under MOBILEGL_IPC_RUN_AHEAD=0 this is a + // call that returns, and the lockstep client's behaviour is unchanged byte for byte + // (G1) - under lockstep appliedSeq is already at LastPublishedSeq by construction. + void QuiesceApplierBeforeFill(const char* surface) { + MG_Remote::Client::ClientSession* session = MG_Remote::Client::ClientSession::Active(); + if (session == nullptr) return; // no session: this process has no applier to outrun + session->WaitForApplyToCatchUp(surface); + } + // Whether the LAST fill actually happened, i.e. whether the rows in the block describe // the verb in flight. Read by MGPipeNoteFrontendMutation, which refreshes one field of // that fill: with no fill behind it there is nothing to refresh and the write would be @@ -637,6 +703,14 @@ namespace MobileGL::MG_Pipe { // P5c (gt, layer 2): the single-field refresh is a client write into gPipeInputs too - // same gate as the fill, and it makes the same claim the fill made: the rows it is // touching belong to a record this thread is parked behind (or will park behind). + // + // P5e (ra2): SO IT TAKES THE SAME WAIT. This one runs at a frontend mutation point, not + // at a verb, so "will park behind" is even weaker here than it is at the fill - the + // mutation can land anywhere between two verbs, with the whole run-ahead backlog in + // flight. The wait is a no-op unless run-ahead is armed, and at a mutation point after a + // barriered verb the applier is usually already caught up, so this is a watermark test + // rather than a park in the common case. + QuiesceApplierBeforeFill("MGPipeNoteFrontendMutation"); MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( "MGPipeNoteFrontendMutation", /*isBarrieredFill=*/true); #endif @@ -3315,6 +3389,17 @@ namespace MobileGL::MG_Pipe { // thread is about to park behind it. The second argument is which of the two // claims the caller is making, and a fill that made the second one falsely is the // named abort. + // + // P5e (ra2): AND IT IS MADE TRUE FIRST. Phase 1 of the block write is three lines + // below - the serial bump, MGPipeServerClearVerbBoundary() and SetVerb - and every + // one of them is a scalar the apply thread reads at each field access inside the + // record it is currently applying. Withdrawing the server's own stamp from under it + // is what produced `Fatal{UnmigratedPipeInput, "@"}` on the apply thread (report §2). THE RED-ONCE IS THIS LINE: + // delete it and the guard below aborts with Fatal{RoleViolation, "gPipeInputs"} + // naming MGPipeValidateForVerb, because the guard now tests the fact rather than + // taking the claim. + QuiesceApplierBeforeFill("MGPipeValidateForVerb"); MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( "MGPipeValidateForVerb", /*isBarrieredFill=*/barriered); } @@ -3630,6 +3715,20 @@ namespace MobileGL::MG_Pipe { // own boundary and §3.3's detector turns any field it still needs from here into // Fatal{UnmigratedPipeInput, "@"} by name, which is what makes the strict // lane a gate rather than a count. +#if MOBILEGL_BUILD_DISAGGREGATED + // P5e (ra2): PHASE 2 OF THE BLOCK WRITE, AND IT NEEDS ITS OWN WAIT. Everything between + // the phase-1 wait and here PUBLISHED RECORDS - the fourteen emitters above - and the + // apply thread reads gPipeInputs while it applies them. So the window the fill opened at + // the top of this function was closed again by this function's own emissions, and the + // 63-field walk below would run straight into it. Same call, same arm, same no-op + // everywhere run-ahead is not armed; the guard beside it is what turns a missing one + // into a named abort rather than a torn read. + if (fillOwed) { + QuiesceApplierBeforeFill("MGPipeValidateForVerb/residual"); + MG_Remote::Client::ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt( + "MGPipeValidateForVerb/residual", /*isBarrieredFill=*/true); + } +#endif const Bool applierDerives = ApplierDerivesRenderStateFields(); // BY VALUE, NOT BY REFERENCE: the memo's storage is rebuilt in place when the key moves, // and the walk below holds this across 63 iterations. diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index d9fcc6dc..eebc405d 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -1316,14 +1316,40 @@ namespace MobileGL::MG_Remote::Client { // // This is where a fill left standing for an unbarriered record lands, by name. if (ClientSessionInstance().RunAheadArmed()) { - if (isBarrieredFill) return; + // ---- P5e (ra2): THE EXEMPTION IS A FACT NOW, NOT A CLAIM ------------------------ + // + // This arm used to read `if (isBarrieredFill) return;`, i.e. the caller's own + // sentence "this touch is the residual fill of a record this thread is about to park + // behind" was accepted as the whole argument. It is an argument about the FUTURE. + // The order at the validate point is fill, then emit, then park, so at the instant + // of the write the apply thread is still draining the unbarriered records the client + // ran ahead of - and the measured consequence was the GL thread bumping + // CurrentVerbSerial, withdrawing MGPipeServerClearVerbBoundary's flag and renaming + // m_currentVerb underneath a record the applier was inside, which surfaced as + // Fatal{UnmigratedPipeInput, "@"} on the apply thread. + // Because the exemption was unconditional, this guard could not fire on any of it: + // that is the answer to "does RefusePipeInputsTouchWhileApplierOwnsIt fire, and if + // not, why not". + // + // So the sentence still exempts the fill, but only once it is TRUE of this instant. + // The fill sites establish it by taking §2.5's forced wait first + // (QuiesceApplierBeforeFill, PipeFill.cpp); removing that wait is the red-once and + // lands here, by name, on the first barriered verb behind a run-ahead backlog. + // + // ApplyThreadIsInsideApplier() is an acquire load of the flag PipeApplier raises for + // the whole of ApplyOne - stamp, decode and LeaveApplier included - so "not inside" + // is exactly "no record is being applied" and not merely "not decoding". + if (isBarrieredFill && !ApplyThreadIsInsideApplier()) return; if (Server::ServerLoop::OnApplyThread()) return; // the applier owns the block MGLOG_F("MGPipe: Fatal{RoleViolation, \"gPipeInputs\"} - the GL thread touched " - "gPipeInputs (%s) on a RUN-AHEAD session outside a barriered fill. With " - "the client running ahead the block is the server's to read for as long " - "as an unbarriered record is in flight (CONTRACT-P5E §3), and this thread " - "has no wait that would make the touch quiescent", - surface); + "gPipeInputs (%s) on a RUN-AHEAD session %s. With the client running ahead " + "the block is the server's to read for as long as ANY record is in flight " + "(CONTRACT-P5E §3), so the only legal GL-thread write is a barriered fill " + "that has first taken §2.5's forced wait (QuiesceApplierBeforeFill)", + surface, + isBarrieredFill ? "in a barriered fill that did not first wait for the " + "applier to catch up" + : "outside a barriered fill"); std::abort(); } // MOBILEGL_IPC_BATCH_WAITS=1 makes "the fill runs while the apply thread applies an diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index a0a0f2c7..7a031bbb 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -208,11 +208,18 @@ namespace MobileGL::MG_Remote::Client { // P5e (ra, CONTRACT-P5E §3.5) RE-DERIVES IT UNDER RUN-AHEAD, and `isBarrieredFill` is // what the caller says about ITSELF: "this touch is the residual fill of a record this // thread is about to park behind". With run-ahead armed the block is server-role - // memory for every other touch, so the rule stops being "not while the applier is - // inside" - which is unanswerable when the client never waits - and becomes "not at - // all, unless you are that fill". The MOBILEGL_IPC_BATCH_WAITS early return goes with + // memory for every other touch. The MOBILEGL_IPC_BATCH_WAITS early return goes with // it: batching is about how many round trips a lockstep client pays, and it says // nothing about a client that pays none. + // + // P5e (ra2) KEEPS THE ARGUMENT AND STOPS TAKING IT ON TRUST. "About to park behind it" + // is a claim about the future; the write is now. Between the fill and the park the + // apply thread is still draining the unbarriered records the client ran ahead of, so an + // unconditional exemption made this guard unfireable on exactly the class it was + // written for - measured as the apply thread aborting on a field whose stamp the GL + // thread had just withdrawn. So the rule is "not while the applier is inside" again, + // for barriered fills too; what changed is that the fill sites MAKE it true first, with + // §2.5's forced wait (PipeFill.cpp's QuiesceApplierBeforeFill), instead of asserting it. static void RefusePipeInputsTouchWhileApplierOwnsIt(const char* surface, Bool isBarrieredFill = false); diff --git a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp index 84f2e2aa..a87e0df3 100644 --- a/MobileGL/MG_Test/Wire/RemoteClientTest.cpp +++ b/MobileGL/MG_Test/Wire/RemoteClientTest.cpp @@ -2297,6 +2297,10 @@ TEST(RemoteGuards, ClientPipeInputsTouchOutsideABarrieredFillUnderRunAheadIsFata // ... and the control: the barriered fill says so and is let through. Without this the case // above would pass just as well against "abort unconditionally". +// +// P5e (ra2): THIS CONTROL IS NOW ALSO HALF OF A PAIR, and the half it does NOT state is the +// one that cost the flip its lane. "The applier is not inside" is true here because nothing in +// this child ever entered it; the case below is the same call with that one fact reversed. TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllowed) { const auto child = RunInChild([] { StartRunAheadSession(); @@ -2307,6 +2311,69 @@ TEST(RemoteGuards, ClientPipeInputsTouchInsideABarrieredFillUnderRunAheadIsAllow ExpectChildSuccess(child); } +// ---- P5e (ra2): THE FLIP'S OWN RED-ONCE, AND THE CLASS THE STRICT LANE CANNOT SEE ---------- +// +// The strict lane runs under LOCKSTEP - ApplyOne stamps every record barriered - so nothing +// that happens only once the client stops waiting is visible to it, by construction (ID-132). +// This pair is what measures it instead, and it runs in the ordinary unit lane on any head, +// flip thrown or not, because StartRunAheadSession arms run-ahead itself. +// +// WHAT IT PINS: `isBarrieredFill` is the caller's sentence "this touch is the residual fill of +// a record this thread is about to park behind". That is a claim about the FUTURE. The order at +// the validate point is fill, then emit, then park, so at the instant of the write the apply +// thread is still draining the UNBARRIERED records the client ran ahead of - and the exemption +// that took the claim on trust made this guard unfireable on precisely the class it exists for. +// Measured on the flipped head: the GL thread bumped CurrentVerbSerial, withdrew the server's +// stamp and renamed m_currentVerb underneath a record the applier was inside, and the applier +// aborted with `Fatal{UnmigratedPipeInput, "@"}` - a verb no applier +// stamp can produce, which is what identifies the writer. +// +// THE PROBE IS THE GUARD CALL ITSELF (ID-102): the flag is raised by the same raw entry point +// PipeApplier's ScopedApplierEntry uses, on the client thread, BEFORE the call under test - so +// what aborts is this probe and not some object built on the wrong thread. +// +// THE RED: in ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt put the run-ahead arm back +// to `if (isBarrieredFill) return;` and this case stops aborting, while the control above keeps +// passing - which is the difference between a rule and an exemption. +TEST(RemoteGuards, ClientBarrieredFillWhileTheApplierIsInsideUnderRunAheadIsFatalByName) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::NoteApplyThreadEnteredApplier(); + ClientSession::RefusePipeInputsTouchWhileApplierOwnsIt("ra-test-surface", + /*isBarrieredFill=*/true); + ClientSession::NoteApplyThreadLeftApplier(); + ClientSessionInstance().Stop(); + }); + ExpectNamedAbort(child, "Fatal{RoleViolation, \"gPipeInputs\"}"); +} + +// ---- P5e (ra2): AND THE OTHER HALF - AN INDEXED DRAW IS AN UNBARRIERED VERB ---------------- +// +// MGP_VERB_OP_LIST joins the whole draw family through ONE row, `DrawVbo -> DrawArrays`, so +// inverting it verb-first answers kOpCount for DrawElements and for every other indexed / +// instanced / multi / indirect verb. The "unknown verb answers barriered" default then made the +// client FILL for each of them while never parking, because the wait is decided per record and +// the record is a draw_vbo (kWaitNone). On the flipped lane that was 21 of the 69 red entries. +// +// The case states it where it is decidable without a GL context: the guard runs before the +// validate point's null-context return, so a verb that answers UNBARRIERED never reaches the +// guard at all and the child exits 0 even with the applier flag up. +// +// THE RED: delete the kDraw fallback in ClientVerbIsBarriered (PipeFill.cpp) and DrawElements +// answers barriered again - the fill runs, the guard above it sees the raised flag, and this +// case aborts with Fatal{RoleViolation, "gPipeInputs"} instead of exiting 0. +TEST(RemoteRunAhead, AnIndexedDrawVerbIsUnbarrieredAndTouchesPipeInputsNotAtAll) { + const auto child = RunInChild([] { + StartRunAheadSession(); + ClientSession::NoteApplyThreadEnteredApplier(); + MGPipeValidateForVerb(MGPipeVerb::DrawElements); + 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 From f286548010184f1516dad61edd305e28bbc2f914 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 13:56:51 -0400 Subject: [PATCH 3/4] [MG_Pipe, MG_Impl, MG_IntegrationTest, CI, Docs] (Disaggregated): P5e ra2 - ID-133's escalation, ID-134's lane, and the contract sentence that could not be checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ID-133: escalation (iii) in MGPipeBarriered - a draw_vbo with NumDraws > 1 that is NOT kDrawIsIndirect is barriered. Espryt's indirect multi-draw tiers reach MultiDrawImpl::RunIndirect, whose apply reads the client's GL_DRAW_INDIRECT_BUFFER binding (BoundDrawIndirectBufferId, MultiDraw.cpp:87) - GetBufferBindingSlot, retiring in P8 - and unbarriered that is Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"} on 18 lane entries. Backtrace of the aborting thread, verbatim frames: CountBarrierPull <- MultiDrawImpl::RunIndirect <- MultiDrawImpl::DrawElementsBatch <- ServerVerbSink::OnDrawVbo <- PipeWireDecoder::DecodeAndApply <- PipeApplier::ApplyOne. - TWO PREMISE CORRECTIONS, both measured, both in the report and in the contract. (a) There is no indirect multi-draw OP: all twenty draw entry points collapse onto draw_vbo, and the tier is resolved on the server per batch (ResolveTierForBatch), so the escalation cannot be keyed on the tier and its cost is NOT confined to the opt-in tier lanes - a plain glMultiDraw* now waits on the auto -> ext default too. NumDraws > 1 is the narrowest wire fact that contains the reaching set; kDrawIsIndirect is excluded so Sodium's indirect draw, which resolves by handle and pulls nothing, keeps running ahead. (b) The read is a SAVE/RESTORE of a GL binding name around the tier's own scratch buffer, not a data dependency - so it can be retired outright by giving BoundDrawIndirectBufferId the handle arm its neighbour ResolveBoundIndexBuffer already has, and this escalation withdrawn. That is three lines in MultiDraw.cpp, which ID-113 gives to mv, so it is reported rather than edited here. - The client's half is verb-keyed because the fill decision is asked before the record exists: {MultiDrawArrays, MultiDrawElements, MultiDrawElementsBaseVertex} is exactly the set whose record can carry NumDraws > 1 without kDrawIsIndirect. The inclusion goes that way round on purpose - a record the server calls barriered whose fields the client did not fill would be an ADMITTED pull of a stale value, which is worse than the abort. - ID-134: the two ClientVertexArrayScenario entries get the label integration-clientarrays-split and an expected-red CI step that asserts the MARKER, shaped like gl's Magma lane. ID-131 heeded by COUNTING, not by assuming: -L integration-split 181 -> 179, -L integration-gpu 1359 -> 1357, the new label selects exactly 2. The step branches on whether run-ahead is actually ARMED, because kMGPipeP5eRunAheadReady is a BUILD constant: on a head where it is false the refusal is unreachable and the entries are legitimately green, so an unconditional demand for red would fail for the one reason that is not a defect. The client says which world it is in in its own log line; delete the green branch when the flip lands for good. - CONTRACT-P5E §3.5 amended to the code: "outside a barriered fill" rested on the caller's claim about the FUTURE, and a guard whose exemption cannot be false on the class it exists for is not a guard - it never fired on any of the 70 red entries. The landed rule is "not while the applier is inside, barriered fill or not", with the fill sites taking §2.5's forced wait first, twice per filling verb because the block is written either side of the emitters. - AND §3.2 IS MARKED UNLANDED, which nobody had noticed: "the server's stamp is server-private … never into the shared block" describes storage the applier owns, and MGPipeServerStampVerbBoundary still writes m_filled / m_currentVerb / m_serverStampedVerb into gPipeInputs itself. That gap is what made the race possible. Splitting the stamp is the P11 item - but per-role stamps WITHOUT versioning the BARRIER_PULLED values would be worse than today, because it turns a loud Fatal into a silent stale read. - §2.1 carries escalation (iii) as a row, per its own rule that a new escalation is a ruling and a row in the contract rather than a condition at a call site. - Gate on the flipped head: unit 2256/2256, isplit 179/179 HARD GREEN, gens rc=0, doc citations rc=0 (the three remaining are CONTRACT-P5.md's and pre-date this branch). --- .github/workflows/test.yml | 60 ++++++++++++++++++++ MobileGL/MG_Impl/Pipe/PipeFill.cpp | 21 +++++++ MobileGL/MG_IntegrationTest/CMakeLists.txt | 23 +++++++- MobileGL/MG_Pipe/PipeApply.cpp | 27 +++++++++ MobileGL/MG_Pipe/PipeApply.h | 7 ++- MobileGL/MG_Remote/CONTRACT-P5E.md | 66 ++++++++++++++++++++++ 6 files changed, 201 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b9bbd91f..3727fffa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1380,6 +1380,66 @@ jobs: fi echo "integration-magma-split is red by design on ${marker}" + # ---- P5e (ra2), ID-134 / ID-82: THE CLIENT-ARRAY LANE, EXPECTED RED UNDER RUN-AHEAD ---- + # + # The two DirectGLES.Split.ClientVertexArrayScenario entries draw from a vertex array in + # the application's OWN memory. The server uploads those bytes by dereferencing a raw + # client pointer per draw, which is exactly the read rule F forbids once the client stops + # waiting - so ID-82 makes the draw a NAMED REFUSAL under run-ahead and gives the staging + # form to P8. Aborting is therefore the DESIGNED behaviour, and a lane that demanded these + # entries green would be demanding that the refusal not fire. + # + # THE LABEL, AND WHY IT IS NOT `integration-split-clientarrays` (ID-131): `ctest -L` is a + # REGEX. That spelling is matched by `-L integration-split` as a substring, so the entries + # would still be in the lane they were moved out of while every listing showed them + # renamed. Counted, not assumed: `-L integration-split` went 181 -> 179 and + # `-L integration-gpu` 1359 -> 1357 when this label landed. + # + # IT BRANCHES ON WHETHER RUN-AHEAD IS ACTUALLY ARMED, and that is not hedging - it is the + # only formulation that is a real assertion on BOTH heads. kMGPipeP5eRunAheadReady is a + # BUILD constant (MG_Backend/Init.cpp): on a head where it is false no environment can arm + # run-ahead, the refusal is unreachable, and these entries are green - so a step that + # simply demanded red would fail for the one reason that is not a defect. The client says + # which world it is in, in its own log line, and the step reads that and then demands the + # matching outcome. When the flip lands for good, delete the green branch. + - name: Integration-clientarrays-split - expected red under run-ahead (ID-82 / ID-134) + working-directory: build-split + env: + MOBILEGL_ITEST_REQUIRE_GPU: "1" + MOBILEGL_TRANSPORT: "inproc" + run: | + ulimit -c unlimited + logs=MobileGL/MG_IntegrationTest/split-logs + rm -f "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null || true + rc=0 + ctest -L integration-clientarrays-split --no-tests=error -j 1 \ + > "${RUNNER_TEMP}/clientarrays.txt" 2>&1 || rc=$? + tail -5 "${RUNNER_TEMP}/clientarrays.txt" + if ! grep -rqF "run-ahead ARMED" "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null; then + # kMGPipeP5eRunAheadReady is false on this head: the client is lockstep, vi's arm is + # alive and these entries are the positive control they were registered as. + if [ "${rc}" -ne 0 ]; then + echo "::error::integration-clientarrays-split is red WITHOUT run-ahead armed. Under lockstep the client-memory array path is legal and vi keeps it working, so this is a real regression in that arm and not ID-82's refusal." + exit 1 + fi + echo "integration-clientarrays-split is green: run-ahead is not armed on this build, so ID-82's refusal is unreachable" + exit 0 + fi + if [ "${rc}" -eq 0 ]; then + echo "::error::integration-clientarrays-split went GREEN with run-ahead ARMED. ID-82 refuses a client-memory vertex array under run-ahead by name, so a green run means the refusal did not fire and the server dereferenced a client pointer from an unbarriered record. If P8 has landed the staged form, delete this step and fold the entries back into integration-split." + exit 1 + fi + marker='Fatal{UnmigratedVerb, "' + if ! grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \ + "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort -u | grep -q CLIENT_ARRAYS; then + echo "::error::integration-clientarrays-split is red but no private log carries ${marker}...+CLIENT_ARRAYS\"} - the lane failed for some OTHER reason, which is a defect rather than the expected-red control" + grep -rhoE '(Fatal|Admitted)\{[A-Za-z]*, "[^"]*"\}' "${logs}"/*ClientVertexArrayScenario*.log 2>/dev/null | sort | uniq -c | sort -rn | head + exit 1 + fi + echo "integration-clientarrays-split is red by design on:" + grep -rhoE 'Fatal\{UnmigratedVerb, "(Multi)?DrawArrays\+CLIENT_ARRAYS"\}' \ + "${logs}"/*ClientVertexArrayScenario*.log | sort -u + - name: Upload strict lane logs if: always() uses: actions/upload-artifact@v7 diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 0c851f34..654eba0f 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -423,6 +423,27 @@ namespace MobileGL::MG_Pipe { 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), ID-133: AND THE CLIENT'S HALF OF ESCALATION (iii) -------------- + // + // The server escalates a PLAIN multi-draw record (`NumDraws > 1` and not indirect) + // because Espryt's indirect tiers still pull the client's GL_DRAW_INDIRECT_BUFFER + // binding at apply. The fill decision is asked HERE, before the record exists, so + // it cannot read NumDraws - it asks the verb instead, and the three verbs below are + // exactly the ones whose record can carry NumDraws > 1 without kDrawIsIndirect. + // + // THE DIRECTION OF THE INEQUALITY IS THE SAFETY ARGUMENT, and it is worth stating + // because the two clauses are spelled differently. The client's set must CONTAIN + // the server's: a record the server calls barriered whose fields the client did not + // fill would be an ADMITTED pull of a stale value - legal-looking and wrong, which + // is worse than the abort. It does contain it, because `NumDraws > 1` can only come + // from a glMultiDraw* entry point, and the `MultiDraw*Indirect*` verbs set + // kDrawIsIndirect and are therefore NOT escalated on either side. The reverse + // inclusion is not needed: a verb the client fills for and the server then calls + // unbarriered costs a fill nobody reads, never a wrong answer. + const Bool plainMultiDraw = verb == MGPipeVerb::MultiDrawArrays || + verb == MGPipeVerb::MultiDrawElements || + verb == MGPipeVerb::MultiDrawElementsBaseVertex; + if (plainMultiDraw) return true; if (op == MGPWireOp::kOpCount && kMGPipeVerbClass[static_cast(verb)] == MGPipeVerbClass::kDraw) { op = MGPWireOp::DrawVbo; diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index 4d57f2de..87e49e2e 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2192,13 +2192,34 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # them by dereferencing a client pointer per draw - and it is therefore both the positive # control for vi's decision to KEEP that arm alive under lockstep and the only entry that # can reach the run-ahead refusal when ra publishes the caps bit. + # + # ---- P5e (ra2), ID-134: AND WITH THE CAPS BIT PUBLISHED IT IS A LANE OF ITS OWN ---------- + # + # "When ra publishes the caps bit" is now, and what these two entries reach is + # `Fatal{UnmigratedVerb, "DrawArrays+CLIENT_ARRAYS"}` / `"MultiDrawArrays+CLIENT_ARRAYS"` - + # ID-82's NAMED REFUSAL, working exactly as ruled. They cannot be green under run-ahead and + # they should not be: a lane that demanded them green would be demanding that the refusal + # not fire, which is the opposite of what vi registered them for. Staging the bytes is P8's. + # + # THE LABEL IS `integration-clientarrays-split` AND THE SPELLING IS THE POINT (ID-131, and + # gl's `integration-magma-split` above is the worked example): `ctest -L` takes a REGEX, so a + # label spelled `integration-split-clientarrays` is still MATCHED by `-L integration-split` + # and the entries would never have left the lane they were moved out of - a split that reads + # as done in every listing and is not. Counted rather than assumed: `ctest -N -L + # integration-split` is 179 after this change and was 181 before, and `-L integration-gpu` + # is 1357 against 1359. `integration-gpu` goes with it for the same reason `integration- + # magma-split` dropped it - a lane that must be green cannot contain an entry that must not. + # + # The two entries keep their private, distinct log paths (SplitLogPaths.cmake.in lists + # MGL_SPLIT_VI_CLIENT_ARRAY_TESTS by name), which is what lets the expected-red CI step + # assert the MARKER rather than merely tolerate a failure. gtest_discover_tests(MobileGLIntegrationTest TEST_PREFIX "DirectGLES.Split." TEST_LIST MGL_SPLIT_VI_CLIENT_ARRAY_TESTS TEST_FILTER "ClientVertexArrayScenario.*" DISCOVERY_TIMEOUT 30 PROPERTIES - LABELS "integration-gpu\;integration-split" + LABELS "integration-clientarrays-split" TIMEOUT ${MGL_ITEST_TIMEOUT} ENVIRONMENT "${MGL_ITEST_GLES_SPLIT_ENVIRONMENT}" ) diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 2fca9c03..5e957459 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -3015,6 +3015,33 @@ namespace MobileGL::MG_Pipe { if (op == MGPWireOp::DrawVbo && payload != nullptr) { const auto& draw = *static_cast(payload); if ((draw.Flags & static_cast(kDrawClientArrays)) != 0) return true; + // 4. Escalation (iii), ID-133: a PLAIN multi-draw - NumDraws > 1 and not indirect. + // Espryt's multi-draw tiers are chosen on the SERVER and per batch + // (MultiDraw.cpp's ResolveTierForBatch), and the two indirect tiers reach + // MultiDrawImpl::RunIndirect, whose apply still reads the client's + // GL_DRAW_INDIRECT_BUFFER binding slot (BoundDrawIndirectBufferId, :87). That + // is a BARRIER_PULLED field whose retiring phase is P8, so under run-ahead an + // unbarriered record reading it is `Fatal{UnmigratedPipeInput, + // "GetBufferBindingSlot@DrawArrays"}` - 18 lane entries on the two tier lanes. + // + // WHY THE KEY IS THE RECORD AND NOT THE TIER, which is the thing a reader will + // want to check: there is exactly ONE draw opcode. Every one of the twenty draw + // entry points collapses onto draw_vbo (MGPDrawInfo's header says so), and the + // tier is resolved on the server, per batch, from driver caps the client does + // not hold - so "escalate the indirect multi-draw op" has no op to name and no + // predicate both roles could compute. NumDraws > 1 is the narrowest wire fact + // that CONTAINS the reaching set: RunIndirect is reachable only from + // DrawElementsBatch, which only a multi-draw record enters. + // + // AND kDrawIsIndirect IS EXCLUDED, which is what keeps Sodium's draw call out of + // it: a genuinely indirect record carries its buffer as a handle in the second + // tail and its apply resolves from that (ResolveIndirectCommandBytes' split + // arm), so it pulls nothing and needs no wait. NumDraws is 0 there, so the test + // is written `> 1` and the flag test is belt to that brace - both are stated + // because the reason each holds is different. + if (draw.NumDraws > 1 && (draw.Flags & static_cast(kDrawIsIndirect)) == 0) { + return true; + } } return false; } diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 502a9a44..3b95ca7d 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -916,10 +916,13 @@ namespace MobileGL::MG_Pipe { // 1. MGPipeWaitClassFor(op) != kWaitNone the static column (§2.2) // 2. kCtxVerb with an open transform-feedback span escalation (i): XFB stays lockstep // 3. draw_vbo carrying kDrawClientArrays escalation (ii): client vertex arrays + // 4. draw_vbo with NumDraws > 1 and NOT indirect escalation (iii): plain multi-draw // // NO OTHER RUNTIME ESCALATION EXISTS. Adding one is an integrator ruling and a row in the - // contract, not a condition somebody adds at a call site - because a fourth clause that only - // one side computes is the same silent failure one level down. + // contract, not a condition somebody adds at a call site - because a fifth clause that only + // one side computes is the same silent failure one level down. (iii) is ID-133's, and + // ID-118's precedent is the one it follows: move the record into a barriered class rather + // than special-case the field its apply still pulls. // // Escalation (ii) is a REFUSAL under run-ahead (§5.1), so on a run-ahead server it never // reaches the sink at all; it is here so the predicate is TOTAL and so the lockstep arm, diff --git a/MobileGL/MG_Remote/CONTRACT-P5E.md b/MobileGL/MG_Remote/CONTRACT-P5E.md index 36a3dac0..48101830 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5E.md +++ b/MobileGL/MG_Remote/CONTRACT-P5E.md @@ -90,6 +90,8 @@ MGPipeBarriered(op, payload, st) = MGPipeWaitClassFor(op) != kWaitNone // static column, §2.2 || (MGPipeCallClassFor(op) == kCtxVerb && st.IsTransformFeedbackActive) // (i) XFB stays lockstep || (op == DrawVbo && (payload.Flags & kDrawClientArrays)) // (ii) client arrays + || (op == DrawVbo && payload.NumDraws > 1 + && !(payload.Flags & kDrawIsIndirect)) // (iii) plain multi-draw ``` `st.IsTransformFeedbackActive` is `MGPContextValues`' value (`MGPipeTypes.h:1698`): on the client it is `ctx.IsTransformFeedbackActive()` at emit, on the server the applied value — `set_context_values` @@ -103,6 +105,37 @@ must not be told a draw is unbarriered on that account. Escalation (ii) is a ref (§5.1), so on a run-ahead server it never reaches the sink; it is listed so the predicate is total. **No other runtime escalation exists**; adding one is an integrator ruling and a row here. +**Escalation (iii) is ID-133's, added by ra2, and it follows ID-118's precedent** — move the +record into a barriered class rather than special-case the field its apply still pulls. Espryt's +multi-draw tier is chosen on the SERVER and per batch (`MultiDraw.cpp`'s `ResolveTierForBatch`), +and the two indirect tiers reach `MultiDrawImpl::RunIndirect`, whose apply still reads the +client's `GL_DRAW_INDIRECT_BUFFER` binding slot (`BoundDrawIndirectBufferId`, `MultiDraw.cpp:87`) +— `GetBufferBindingSlot`, whose retiring phase is P8. Unbarriered, that is +`Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}`: 18 lane entries on the two +tier-pinned lanes. + +**Why the key is the RECORD and not the tier.** There is exactly one draw opcode — all twenty +draw entry points collapse onto `draw_vbo` — and the tier is resolved on the server from driver +caps the client does not hold, so "escalate the indirect multi-draw op" has no op to name and no +predicate both roles could compute. `NumDraws > 1` is the narrowest wire fact that CONTAINS the +reaching set (`RunIndirect` is reachable only from `DrawElementsBatch`, which only a multi-draw +record enters). `kDrawIsIndirect` is excluded because a genuinely indirect record carries its +buffer as a handle in the second tail and resolves from that, pulling nothing. **The cost is +therefore NOT confined to the opt-in tier lanes**: a plain `glMultiDraw*` now waits on every tier, +including the `auto` → `ext` default. That is a real charge against §0's goal and the device arm +owns measuring it. **It can be withdrawn outright** once `BoundDrawIndirectBufferId` takes the +handle arm its own neighbour `ResolveBoundIndexBuffer` already has (three lines, `MultiDraw.cpp`, +mv's file per ID-113): the read is a SAVE/RESTORE of a GL binding name around the tier's scratch +buffer, not a data dependency, so the server can answer it from its own binding record and the +pull disappears rather than being legalised. + +The client's half is asked at the validate point, before the record exists, so it cannot read +`NumDraws`; it asks the verb, and `{MultiDrawArrays, MultiDrawElements, MultiDrawElementsBaseVertex}` +is exactly the set whose record can carry `NumDraws > 1` without `kDrawIsIndirect`. **The +inclusion must go that way round**: a record the server calls barriered whose fields the client +did not fill would be an ADMITTED pull of a stale value, which is worse than the abort; a verb the +client fills for and the server then calls unbarriered costs only a fill nobody reads. + ### 2.2 The static column, row by row | class | rows | why | @@ -212,6 +245,39 @@ UNBARRIERED apply is a finding: `MGLOG_E_ONCE` with the kind, Fatal under strict outside a barriered fill is `Fatal{RoleViolation, "gPipeInputs"}` by name. `Fatal{BarrierViolation}` (`:840`) stays for `BatchWaits==0 && !RunAhead`. + **AMENDED by ra2 (ID-132, and it is the sentence this section got wrong).** "Outside a + barriered fill" was the whole test, and a barriered fill was exempted on the strength of what + the caller SAID about itself: *"this touch is the residual fill of a record this thread is + about to park behind"*. That is a claim about the **future**, and the write is in the + **present** — the order at the validate point is fill, then emit, then park, so at the instant + of the write the apply thread is still draining the unbarriered records the client ran ahead + of. **A guard whose exemption cannot be false on the class it exists for is not a guard.** It + never fired on any of the 70 red lane entries; what fired instead was the apply thread, on + `Fatal{UnmigratedPipeInput, "@"}`, after the GL thread had bumped + `CurrentVerbSerial`, withdrawn `m_serverStampedVerb` and renamed `m_currentVerb` underneath a + record it was inside. + + **The rule as landed:** under `RunAheadArmed` a GL-thread write to the block is legal only + while `ApplyThreadIsInsideApplier()` is false, barriered fill or not; and the three fill sites + (`MGPipeValidateForVerb` phase 1, its step-4 residual walk, `MGPipeNoteFrontendMutation`) + establish that by taking §2.5's forced wait first (`QuiesceApplierBeforeFill`, + `PipeFill.cpp`). **Two waits per filling verb, not one**, because the block is written in two + phases that straddle publication — the serial bump / stamp withdrawal / verb rename before the + emitters, the 63-field walk after them — and the records those emitters published are records + whose apply reads the block. The red-once is deleting a wait; the guard then aborts by name on + the first filling verb behind a run-ahead backlog. Pinned by + `RemoteGuards.ClientBarrieredFillWhileTheApplierIsInsideUnderRunAheadIsFatalByName` with its + green control beside it; both run in the `unit` lane, because the strict lane runs under + lockstep and cannot see this class by construction. + + The same correction applies to **§3.2 above, which is UNLANDED and was never marked as such**: + "the server's stamp is server-private … never into the shared block" describes storage the + applier owns, and `MGPipeServerStampVerbBoundary` still writes `m_filled` / `m_currentVerb` / + `m_serverStampedVerb` into `gPipeInputs` itself. That gap is exactly what made the race + possible, and splitting the stamp is the P11 item — but note that per-role stamps **without** + versioning the BARRIER_PULLED values would be worse than today, because it turns a loud + `Fatal{UnmigratedPipeInput}` into a silent stale read. Both, or the values retired first. + --- ## §4 id — handle-keyed twins From efe58eac146b13dedf4d38476db47d3926bbbaa7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 14:10:04 -0400 Subject: [PATCH 4/4] [MG_Backend, MG_Pipe, MG_Impl, Docs] (Disaggregated): P5e ra2 ID-136 - retire the indirect-binding pull instead of escalating for it, and withdraw escalation (iii) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - TOUCHING mv's FILE UNDER ID-136, deliberately and by ruling: MG_Backend/DirectGLES/ MultiDraw.cpp is mv's per ID-113, and the integrator overrode that boundary for this one change because the two halves must not exist apart on any head. The boundary has not drifted. - BoundDrawIndirectBufferId (MultiDraw.cpp) gets the handle arm its neighbour ResolveBoundIndexBuffer already had, in mv's own idiom rather than a second one beside it: the arm is `MG_Config::Transport != Monolith`, STATED (ID-81) and never inferred from a null handle (ID-110); a null handle is "this verb bound no indirect buffer", the same 0 the monolith arm returns for an empty slot; a missing backend resource is the named refusal Fatal{RoleViolation, "multidraw-indirect-buffer-arm"}, a sibling of mv's RefuseMissingIndexBufferRecord, never a quiet fall-back to the frontend. The monolith body below it is unchanged token for token. The function moved down beside BoundIndexBufferId, which is the same question about the other target. - WHY A HANDLE ARM AND NOT A MIGRATION: the tier never reads a byte of the client's indirect buffer. It binds its OWN scratch command buffer and wants to put back the name that was there - a save/restore. With a transport the only other writer of this process's GL_DRAW_INDIRECT_BUFFER is DirectGLES.cpp's DrawSyncBit::IndirectBuffer arm, which binds from MGPipeApplier().VerbIndirectBuffer and nothing else, so "what was bound" IS "what the verb's record named". - Escalation (iii) withdrawn in the same commit - MGPipeBarriered's clause, the client's verb-keyed half in ClientVerbIsBarriered, and the contract row. It was never wrong, it was the wrong instrument: there is one draw opcode and the tier is chosen server-side per batch, so the narrowest key both roles could compute charged EVERY plain glMultiDraw* on the auto -> ext default - a per-batch rendezvous on the arm the phone ships, added to make a lane green. THE CHECK ON AN ESCALATION IS "WHICH ARM PAYS FOR IT", NOT "WHICH LANE GOES GREEN"; the rule is recorded in PipeApply.h beside the three that remain and in CONTRACT-P5E §2.1. - Red-once D: `if (false && ...)` on the new arm and the entry dies 3/3, deterministically, on `Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"} [BARRIER-PULLED, UNBARRIERED, the client did not fill it, retires in P8 (indirect), P9 (readback), P13 (transfer)]`. Its counterpart is this head itself: the escalation is GONE and the lane is green, so nothing needed it - and the strict census's admitted set drops from nine rows back to ID-128's eight, because the pull no longer happens at all rather than being legalised. - Gate on the flipped head: unit 2256/2256, isplit 179/179, gens rc=0, lane counts unchanged (179 / 1357 / 2 / 2). --- MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp | 78 ++++++++++++++++++-- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 30 +++----- MobileGL/MG_Pipe/PipeApply.cpp | 44 +++++------ MobileGL/MG_Pipe/PipeApply.h | 15 ++-- MobileGL/MG_Remote/CONTRACT-P5E.md | 53 ++++++------- 5 files changed, 132 insertions(+), 88 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp b/MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp index 7229ec79..c8f0060c 100644 --- a/MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp +++ b/MobileGL/MG_Backend/DirectGLES/MultiDraw.cpp @@ -84,13 +84,8 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl { constexpr SizeT kMaxComputeWorkGroups = 65535; constexpr SizeT kMaxComputeFlattenedIndices = kMaxComputeWorkGroups * kComputeWorkGroupSize; - Uint BoundDrawIndirectBufferId() { - const auto& indirect = - MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject(); - if (!indirect) return 0; - const auto* resource = BufferImpl::EnsureBufferResource(indirect); - return resource ? resource->id : 0; - } + // BoundDrawIndirectBufferId MOVED (P5e ra2, ID-136) to sit beside BoundIndexBufferId, + // which is the same question about the other target and now has the same two arms. // --------------------------------------------------------------------------- // The bound index buffer, and WHICH SIDE ANSWERS FOR IT @@ -150,6 +145,24 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl { std::abort(); } + // ---- P5e (ra2), ID-136: THE SAME REFUSAL FOR THE INDIRECT TARGET ------------------- + // + // A sibling of mv's rather than a second idiom, because it is the same sentence about + // the other buffer target: a missing record on the handle arm aborts by name and never + // falls back to the frontend binding slot, which answers for whatever the CLIENT has + // bound NOW - a later verb than the one being applied. + [[noreturn]] void RefuseMissingIndirectBufferRecord(const char* entry, + MG_Pipe::MGPipeHandle res) { + MGLOG_F("MGPipe: Fatal{RoleViolation, \"multidraw-indirect-buffer-arm\"} - %s found no " + "backend resource for the indirect buffer {%u, %u} this verb's record named. " + "The indirect buffer of this family is MGPipeApplier().VerbIndirectBuffer " + "(CONTRACT-P5E §2.1) and the arm is selected by Transport != Monolith " + "(ID-81); falling back to the frontend GL_DRAW_INDIRECT_BUFFER slot here " + "would restore whatever the CLIENT has bound now", + entry, res.Slot, res.Gen); + std::abort(); + } + // Does the applier's record say the application SUPPLIED this store's content? The M-3 // rule ScopedRestartIndexSubstitution spells: for a supplied store a coverage gap in the // server shadow is a missing record and Fatal by name; for one the application ORPHANED @@ -247,6 +260,57 @@ namespace MobileGL::MG_Backend::DirectGLES::MultiDrawImpl { return ResolveBoundIndexBuffer(IndexBufferQuestion::DriverName, "BoundIndexBufferId").Id; } + // The GL name on GL_DRAW_INDIRECT_BUFFER, i.e. what a tier that swaps in its own scratch + // COMMAND buffer has to put back - the exact twin of BoundIndexBufferId above, and now + // with the same two arms. + // + // ---- P5e (ra2), ID-136: THE SEAT THIS FILE'S OWN RULE HAD MISSED ------------------- + // + // This was the LAST unguarded frontend read on the multi-draw apply path, and it sat + // twenty lines above the function that retired its neighbour. It asked + // `MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect)` and then + // `EnsureBufferResource()` - a registry lookup keyed by the client's + // identity, which is §4.4's rule and not only the allocator's. Under run-ahead that is + // `Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}` on every batch the + // indirect tiers execute: 18 lane entries, and the only thing standing between the flip + // and a green lane once the fill race was fixed. + // + // WHAT IT IS NOT: a data dependency. The tier does not want the client's indirect + // buffer - it never reads a byte of it. It binds its OWN scratch command buffer + // (g_indirectCommands) and wants to put back the name that was there. So the answer is + // not "migrate the value" (which is what P8 owes for the ordinary indirect draw path); + // it is "ask the side that did the binding". ID-133's escalation (iii) legalised the + // pull instead, at the price of a rendezvous on every plain glMultiDraw* on the DEFAULT + // tier - a real cost on the shipping arm, paid to make a lane green. ID-136 withdraws + // that escalation and retires the read, in the commit that adds this arm. + // + // WHY MGPipeApplier().VerbIndirectBuffer IS THE WHOLE ANSWER ON THIS ARM: with a + // transport, the ONLY writer of this process's GL_DRAW_INDIRECT_BUFFER outside this + // function is DirectGLES.cpp's DrawSyncBit::IndirectBuffer arm, which binds from exactly + // that handle and nothing else. So "what was bound" IS "what the verb's record named", + // and a null handle is "this verb bound none" - the same 0 the monolith arm returns for + // an empty slot, and not the arm test (ID-110: the arm was decided by the transport + // above, never inferred from a null). + Uint BoundDrawIndirectBufferId() { +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + const MG_Pipe::MGPipeHandle res = MG_Pipe::MGPipeApplier().VerbIndirectBuffer; + if (MG_Pipe::MGPipeHandleIsNull(res)) return 0; + auto* resource = BufferImpl::EnsureBufferResourceForHandle(nullptr, res); + if (resource == nullptr) { + RefuseMissingIndirectBufferRecord("BoundDrawIndirectBufferId", res); + } + return resource->id; + } +#endif + // MONOLITH GLUE from here down, token for token what this function did before. + const auto& indirect = + MGB_CTX->GetBufferBindingSlot(BufferTarget::DrawIndirect).GetBoundObject(); + if (!indirect) return 0; + const auto* resource = BufferImpl::EnsureBufferResource(indirect); + return resource ? resource->id : 0; + } + // --------------------------------------------------------------------------- // Scratch GL objects // diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 654eba0f..72ff9c24 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -423,27 +423,15 @@ namespace MobileGL::MG_Pipe { 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), ID-133: AND THE CLIENT'S HALF OF ESCALATION (iii) -------------- - // - // The server escalates a PLAIN multi-draw record (`NumDraws > 1` and not indirect) - // because Espryt's indirect tiers still pull the client's GL_DRAW_INDIRECT_BUFFER - // binding at apply. The fill decision is asked HERE, before the record exists, so - // it cannot read NumDraws - it asks the verb instead, and the three verbs below are - // exactly the ones whose record can carry NumDraws > 1 without kDrawIsIndirect. - // - // THE DIRECTION OF THE INEQUALITY IS THE SAFETY ARGUMENT, and it is worth stating - // because the two clauses are spelled differently. The client's set must CONTAIN - // the server's: a record the server calls barriered whose fields the client did not - // fill would be an ADMITTED pull of a stale value - legal-looking and wrong, which - // is worse than the abort. It does contain it, because `NumDraws > 1` can only come - // from a glMultiDraw* entry point, and the `MultiDraw*Indirect*` verbs set - // kDrawIsIndirect and are therefore NOT escalated on either side. The reverse - // inclusion is not needed: a verb the client fills for and the server then calls - // unbarriered costs a fill nobody reads, never a wrong answer. - const Bool plainMultiDraw = verb == MGPipeVerb::MultiDrawArrays || - verb == MGPipeVerb::MultiDrawElements || - verb == MGPipeVerb::MultiDrawElementsBaseVertex; - if (plainMultiDraw) return true; + // 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; diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 5e957459..c10c8153 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -3015,33 +3015,27 @@ namespace MobileGL::MG_Pipe { if (op == MGPWireOp::DrawVbo && payload != nullptr) { const auto& draw = *static_cast(payload); if ((draw.Flags & static_cast(kDrawClientArrays)) != 0) return true; - // 4. Escalation (iii), ID-133: a PLAIN multi-draw - NumDraws > 1 and not indirect. - // Espryt's multi-draw tiers are chosen on the SERVER and per batch - // (MultiDraw.cpp's ResolveTierForBatch), and the two indirect tiers reach - // MultiDrawImpl::RunIndirect, whose apply still reads the client's - // GL_DRAW_INDIRECT_BUFFER binding slot (BoundDrawIndirectBufferId, :87). That - // is a BARRIER_PULLED field whose retiring phase is P8, so under run-ahead an - // unbarriered record reading it is `Fatal{UnmigratedPipeInput, - // "GetBufferBindingSlot@DrawArrays"}` - 18 lane entries on the two tier lanes. + // ---- ESCALATION (iii) WAS HERE AND IS WITHDRAWN (P5e ra2, ID-133 then ID-136) --- // - // WHY THE KEY IS THE RECORD AND NOT THE TIER, which is the thing a reader will - // want to check: there is exactly ONE draw opcode. Every one of the twenty draw - // entry points collapses onto draw_vbo (MGPDrawInfo's header says so), and the - // tier is resolved on the server, per batch, from driver caps the client does - // not hold - so "escalate the indirect multi-draw op" has no op to name and no - // predicate both roles could compute. NumDraws > 1 is the narrowest wire fact - // that CONTAINS the reaching set: RunIndirect is reachable only from - // DrawElementsBatch, which only a multi-draw record enters. + // ID-133 escalated a PLAIN multi-draw (`NumDraws > 1 && !kDrawIsIndirect`) so that + // MultiDrawImpl::RunIndirect's read of the client's GL_DRAW_INDIRECT_BUFFER binding + // became a legal barriered pull instead of an unbarriered Fatal. It worked, and it + // cost a rendezvous on EVERY plain glMultiDraw* on the DEFAULT tier: there is one + // draw opcode - all twenty entry points collapse onto draw_vbo - and the tier is + // chosen on the server per batch (MultiDraw.cpp's ResolveTierForBatch), so no + // predicate both roles can compute names the arm that actually needed it. // - // AND kDrawIsIndirect IS EXCLUDED, which is what keeps Sodium's draw call out of - // it: a genuinely indirect record carries its buffer as a handle in the second - // tail and its apply resolves from that (ResolveIndirectCommandBytes' split - // arm), so it pulls nothing and needs no wait. NumDraws is 0 there, so the test - // is written `> 1` and the flag test is belt to that brace - both are stated - // because the reason each holds is different. - if (draw.NumDraws > 1 && (draw.Flags & static_cast(kDrawIsIndirect)) == 0) { - return true; - } + // THE CHECK ON AN ESCALATION IS "WHICH ARM PAYS FOR IT", NOT "WHICH LANE GOES + // GREEN", and that is the rule this pair of rulings exists to record. The read was + // a save/restore of a GL binding NAME around the tier's own scratch buffer, not a + // data dependency, so the answer was to ask the side that did the binding: + // BoundDrawIndirectBufferId now takes the handle arm its neighbour + // ResolveBoundIndexBuffer already had, and the pull is GONE rather than legalised. + // + // Both halves landed in ONE commit on purpose: the pull retired without this clause + // withdrawn is a cost with no reason, and this clause withdrawn without the pull + // retired puts 18 lane entries back on the unbarriered arm. Neither is a state to + // gate or to measure, so neither was ever a head. } return false; } diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 3b95ca7d..7d41d405 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -916,13 +916,18 @@ namespace MobileGL::MG_Pipe { // 1. MGPipeWaitClassFor(op) != kWaitNone the static column (§2.2) // 2. kCtxVerb with an open transform-feedback span escalation (i): XFB stays lockstep // 3. draw_vbo carrying kDrawClientArrays escalation (ii): client vertex arrays - // 4. draw_vbo with NumDraws > 1 and NOT indirect escalation (iii): plain multi-draw // // NO OTHER RUNTIME ESCALATION EXISTS. Adding one is an integrator ruling and a row in the - // contract, not a condition somebody adds at a call site - because a fifth clause that only - // one side computes is the same silent failure one level down. (iii) is ID-133's, and - // ID-118's precedent is the one it follows: move the record into a barriered class rather - // than special-case the field its apply still pulls. + // contract, not a condition somebody adds at a call site - because a fourth clause that only + // one side computes is the same silent failure one level down. + // + // THERE WAS AN ESCALATION (iii) AND IT WAS WITHDRAWN, which is worth a line here rather than + // only in the history: ID-133 barriered a plain multi-draw to legalise a pull in Espryt's + // indirect multi-draw tier, ID-136 retired the pull instead (MultiDraw.cpp's + // BoundDrawIndirectBufferId) and took the clause back out. The rule it leaves behind is the + // one to apply to the next candidate: ask WHICH ARM PAYS for an escalation, not which lane + // it turns green - (iii) was keyed on the only wire fact available and therefore charged the + // default tier for a read only two opt-in tiers made. // // Escalation (ii) is a REFUSAL under run-ahead (§5.1), so on a run-ahead server it never // reaches the sink at all; it is here so the predicate is TOTAL and so the lockstep arm, diff --git a/MobileGL/MG_Remote/CONTRACT-P5E.md b/MobileGL/MG_Remote/CONTRACT-P5E.md index 48101830..d87f3814 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5E.md +++ b/MobileGL/MG_Remote/CONTRACT-P5E.md @@ -90,8 +90,6 @@ MGPipeBarriered(op, payload, st) = MGPipeWaitClassFor(op) != kWaitNone // static column, §2.2 || (MGPipeCallClassFor(op) == kCtxVerb && st.IsTransformFeedbackActive) // (i) XFB stays lockstep || (op == DrawVbo && (payload.Flags & kDrawClientArrays)) // (ii) client arrays - || (op == DrawVbo && payload.NumDraws > 1 - && !(payload.Flags & kDrawIsIndirect)) // (iii) plain multi-draw ``` `st.IsTransformFeedbackActive` is `MGPContextValues`' value (`MGPipeTypes.h:1698`): on the client it is `ctx.IsTransformFeedbackActive()` at emit, on the server the applied value — `set_context_values` @@ -105,36 +103,31 @@ must not be told a draw is unbarriered on that account. Escalation (ii) is a ref (§5.1), so on a run-ahead server it never reaches the sink; it is listed so the predicate is total. **No other runtime escalation exists**; adding one is an integrator ruling and a row here. -**Escalation (iii) is ID-133's, added by ra2, and it follows ID-118's precedent** — move the -record into a barriered class rather than special-case the field its apply still pulls. Espryt's -multi-draw tier is chosen on the SERVER and per batch (`MultiDraw.cpp`'s `ResolveTierForBatch`), -and the two indirect tiers reach `MultiDrawImpl::RunIndirect`, whose apply still reads the -client's `GL_DRAW_INDIRECT_BUFFER` binding slot (`BoundDrawIndirectBufferId`, `MultiDraw.cpp:87`) -— `GetBufferBindingSlot`, whose retiring phase is P8. Unbarriered, that is -`Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}`: 18 lane entries on the two -tier-pinned lanes. +**There was an escalation (iii) and it was WITHDRAWN — ID-133 added it, ID-136 took it back out, +and the reason is the most reusable thing P5e learned about escalations.** ID-133 barriered a +plain multi-draw (`NumDraws > 1 && !kDrawIsIndirect`) so that `MultiDrawImpl::RunIndirect`'s read +of the client's `GL_DRAW_INDIRECT_BUFFER` binding became a legal barriered pull instead of +`Fatal{UnmigratedPipeInput, "GetBufferBindingSlot@DrawArrays"}` on 18 lane entries. It worked, and +it was the wrong instrument: -**Why the key is the RECORD and not the tier.** There is exactly one draw opcode — all twenty -draw entry points collapse onto `draw_vbo` — and the tier is resolved on the server from driver -caps the client does not hold, so "escalate the indirect multi-draw op" has no op to name and no -predicate both roles could compute. `NumDraws > 1` is the narrowest wire fact that CONTAINS the -reaching set (`RunIndirect` is reachable only from `DrawElementsBatch`, which only a multi-draw -record enters). `kDrawIsIndirect` is excluded because a genuinely indirect record carries its -buffer as a handle in the second tail and resolves from that, pulling nothing. **The cost is -therefore NOT confined to the opt-in tier lanes**: a plain `glMultiDraw*` now waits on every tier, -including the `auto` → `ext` default. That is a real charge against §0's goal and the device arm -owns measuring it. **It can be withdrawn outright** once `BoundDrawIndirectBufferId` takes the -handle arm its own neighbour `ResolveBoundIndexBuffer` already has (three lines, `MultiDraw.cpp`, -mv's file per ID-113): the read is a SAVE/RESTORE of a GL binding name around the tier's scratch -buffer, not a data dependency, so the server can answer it from its own binding record and the -pull disappears rather than being legalised. +- There is exactly **one draw opcode** — all twenty draw entry points collapse onto `draw_vbo` — + and the multi-draw tier is resolved on the SERVER, per batch (`MultiDraw.cpp`'s + `ResolveTierForBatch`), from driver caps the client does not hold. So "escalate the indirect + multi-draw op" has no op to name and no predicate both roles can compute, and the narrowest + available key charged **every plain `glMultiDraw*` on every tier**, including the `auto` → `ext` + default the phone ships — a per-batch rendezvous on the shipping arm, added to satisfy a lane. +- And the read was never a data dependency: `BoundDrawIndirectBufferId` SAVES AND RESTORES a GL + binding name around the tier's own scratch command buffer. Giving it the handle arm its + neighbour `ResolveBoundIndexBuffer` already had retires the pull outright. -The client's half is asked at the validate point, before the record exists, so it cannot read -`NumDraws`; it asks the verb, and `{MultiDrawArrays, MultiDrawElements, MultiDrawElementsBaseVertex}` -is exactly the set whose record can carry `NumDraws > 1` without `kDrawIsIndirect`. **The -inclusion must go that way round**: a record the server calls barriered whose fields the client -did not fill would be an ADMITTED pull of a stale value, which is worse than the abort; a verb the -client fills for and the server then calls unbarriered costs only a fill nobody reads. +**The rule this leaves for the next candidate escalation: ask WHICH ARM PAYS FOR IT, not which +lane it turns green.** A wait added to make a lane green is a real cost on a real path; if the +predicate cannot name the arm that needs it, the escalation is charging arms that do not. + +The two halves — retiring the pull and withdrawing the clause — landed in one commit (ID-136 +overrode ID-113's package boundary for it): the pull retired without the clause withdrawn is a +cost with no reason, and the clause withdrawn without the pull retired puts the 18 entries back +on the unbarriered arm. ### 2.2 The static column, row by row