Files

12 KiB

TASK-gl — make the strict lane mean something, and make the flip survivable

Read PACKAGE-PREAMBLE-P5E.md and PREAMBLE-ADDENDUM-WAVE3.md first. Your tree is ~/w7/p5e-gl, branch p5e/gl, base 7c6f6886. Your slug for the gate driver is gl.

You are not migrating a field family. You are fixing the mechanism that decides whether P5e is finished, plus two defects that would break other backends the moment the phase's flags flip. Two sibling packages are running concurrently (pa on the program family in ~/w7/p5e-pa, mv on multi-draw in ~/w7/p5e-mv). Stay out of their trees and out of MG_Backend/DirectGLES/DirectGLES.cpp and MultiDraw.cpp except where this brief names a line.

Do the items in the order given. Items 1 and 2 are correctness landmines and must land even if you run out of room for the rest; commit each item separately.


Item 1 — the barriered stamp must ask the server's own capability (ruling ID-111)

This is a latent crash, not a cleanup. MGPipeRunAheadCapBitsFor (MG_Pipe/MGPipeTypes.h:160-166) never publishes kCapRunAheadApply on BackendType::DirectVulkan, because ruling ID-90 keeps Magma in lockstep for all of P5e. But the server-side stamp in PipeApplier::ApplyOne (MG_Remote/Server/PipeApplier.cpp:1180-1182) reads only the build constant:

MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(
    MG_Pipe::kMGPipeP5eClientWaitRuleLanded ? wireSaysBarriered : true);

draw_vbo, blit, clear and launch_grid are all kWaitNone. So the moment kMGPipeP5eClientWaitRuleLanded flips, a Magma server stamps every draw record UNBARRIERED while its client is still lockstep — and CountBarrierPull (MG_Backend/MGPipe/PipeInputs.cpp:95-97) aborts unconditionally on an unbarriered pull, no knob involved. Magma dies on its first draw, and MagmaP7AllocatorDebtScope's exemption is withdrawn with it.

Fix: conjoin the server's own capability. ServerPublishesRunAhead() already exists at PipeApplier.cpp:40-44 and answers false for a session with no CallMask yet, which is the right answer for the bring-up window.

MG_Pipe::MGPipeApplierSetCurrentRecordBarriered(
    (MG_Pipe::kMGPipeP5eClientWaitRuleLanded && ServerPublishesRunAhead()) ? wireSaysBarriered : true);

Keep wireSaysBarriered computed unconditionally — that is ID-103's reason, the predicate must stay exercised for the whole phase rather than first run on the day it starts deciding.

Red-once: a RemoteClientTest case that drives a DrawVbo through ApplyOne on a session whose CallMask lacks bit 10 and asserts MGPipeApplierCurrentRecordIsBarriered() is true. Removing the new conjunct makes it false. This is the server-side mirror of the client latch and it is what makes ID-90 true in code rather than in prose.

Item 2 — a compile-time trip wire on the residual fill (ruling ID-112)

EmittedCallSuppliesTheWholeField (MG_Impl/Pipe/PipeFill.cpp:2499-2519) is Magma's only source for the seven pointer-backed fields, and Magma dereferences them immediately. Two of the seven rows, GetBoundVertexArray and GetBufferBindingPoint, have no compile-time protection at all: a package that "tidies up" by removing one would not break the build, it would SIGSEGV Magma on its first draw on a device.

Fix: widen NoP4aFamilyFieldIsWhollySupplied (PipeFill.cpp:2667-2682) to cover all seven rows, or add a second static_assert naming the two unprotected ones. About ten lines. This converts the most likely mistake of every remaining package from a device crash into a build break, which is why it is item 2 and not item 8.

Red-once: delete the GetBoundVertexArray row from the table and show the build fails with your named assertion.

Item 3 — the admission predicate, GENERATED (ruling ID-116)

BRIEF §3.3's "§7 allowlist" exists, in MobileGL/MG_Remote/CONTRACT-P5E.md:475-486. Package ra also landed a hand-kept 13-row copy in .github/workflows/test.yml:1207-1292. That copy is wrong in both directions and must be deleted, not corrected.

The rule, which is ID-84 expressed in tables that already exist: a <field>@<verb> pair is admitted iff the field's row is BARRIER_PULLED and the verb's wire op is statically barriered (MGPipeWaitClassFor(op) != kWaitNone), intersected with kMGPipeClassFieldMask[kMGPipeVerbClass[verb]].

Fix: extend scripts/gen_pipe_field_ownership.py — it already joins FieldOwnership.def, already emits MG_Pipe/generated/PipeFieldOwnership.inc, and already has --check / --self-test wired into the gens gate step — to emit

inline constexpr MGPipeFieldMask kMGPipeAdmittedPullMask[kMGPipeVerbCount] = { … };
constexpr Bool MGPipeBarrierPullAdmitted(MGPipeInputField f, MGPipeVerb v) {
    return MGPipeFieldMaskHas(kMGPipeAdmittedPullMask[SizeT(v)], f);
}

from kMGPipeFieldOwnership, MGP_VERB_OP_LIST (FieldOwnership.def:255) and kMGPipeWaitClasses (generated/PipeWire.inc:376). Add --print-admitted so the shell reads the same table the C++ does. Extend --self-test with negative controls in the style already there.

By this rule GetFramebufferBindingSlot@ReadPixels (21 entries, the largest survivor) is admitted, and GetTextureObject@CopyImageSubData is not — see item 5.

Item 4 — the strict knob becomes admission-aware (ruling ID-117)

Today CountBarrierPull aborts on every barrier-pulled read under MOBILEGL_IPC_STRICT_ERRORS, admitted or not, so the CI's allowlist comparison is unreachable code and "hard green" is impossible with the knob as written. Give it a third state, leaving the unbarriered arm exactly as it is:

if (!MGPipeApplierCurrentRecordIsBarriered())
    StrictBarrierPullFatal(field, verb, "UNBARRIERED, the client did not fill it");   // unchanged
++g_residualPulls; …
if (MG_Config::Ipc.StrictErrors) {
    if (!MGPipeBarrierPullAdmitted(field, verb))
        StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1");          // unchanged
    AdmittedBarrierPullOnce(field, verb);   // MGLOG_W, deduped per (field, verb)
}

The admitted line keeps the marker grammar and changes only the leading tag:

MGPipe: Admitted{UnmigratedPipeInput, "<F>@<V>"} [BARRIER-PULLED, ADMITTED, retires in <phase>]

so every filter written since P5c that matches Fatal{UnmigratedPipeInput keeps meaning exactly "red". The dedupe is required: an 852-draw frame must not write 852 lines.

Check, do not assume, that FieldOwnershipTest.StrictErrorsTurnsABarrierPulledReadIntoANamedAbort and StrictErrorsAlsoPromotesTheStickyForwards still go red — both stamp MGPipeVerb::DrawArrays, whose op is kWaitNone, so they should.

Item 5 — ResourceCopyRegion becomes barriered (ruling ID-118)

GetTextureObject retires in P7, not P5e, but ResourceCopyRegion is kWaitNone (generated/PipeWire.inc:412). After the flip that is a genuinely unbarriered record reading client memory: an unconditional Fatal, no knob. The ruling is to move the op to a barriered wait class rather than special-case the field into the allowlist — then item 3's derivation admits it with no exception, and P7 can retire the field on its own schedule. CopyImageSubData is not on the per-frame path (zero per frame in the Minecraft scene), so the wait costs nothing measurable. Say in your report which wait class you chose and why.

Item 6 — the lane reads every log, and the rsp pin is reworded (rulings ID-119, ID-115)

Three defects, all measured:

  1. The CI step greps MobileGL/MG_IntegrationTest/split-logs/, which holds 90 of the lane's 116 entries. The 26 it misses are exactly the F1. readback block, both NamedBlit pairs, the four Ct. entries and PersistentMapArm — i.e. precisely the readback population the allowlist is about. Drive the log set from ctest instead of from a directory: split_log_paths.py:12-33 already parses ctest --show-only=json-v1 into {entry: absolute MOBILEGL_LOG_FILE_PATH}. Add a markers mode that walks that map and classifies every Fatal{ and Admitted{ line, and make the lane a two-sided ratchet: an unadmitted marker fails, and an admitted pair that no longer appears must be removed from the expected set (so the lane cannot rot green).
  2. The rsp check greps for a counter no split lane emits. And the pin itself is vacuous: the unbarriered arm of CountBarrierPull is [[noreturn]] and runs before ++g_residualPulls, so rsp counts only barriered pulls and "rsp = 0 on unbarriered records" is true whatever the code does. Reword it to what is checkable: rsp counts only admitted barriered pulls and must equal the admitted-marker tally. Assert rsp against draws rather than against 0 — on the device rsp equalled the draw count under inproc and 0 under monolith, so the draw path's retirement is visible as rsp ceasing to scale with draws. MG_IntegrationTest/Harness/PipeStatsWindow.h already gives you CounterOrAbsent(window, "rsp") and "draws"; six scenarios already use it. You need one counting entry in the split lane with MOBILEGL_PIPE_STATS=1, MOBILEGL_PIPE_STATS_PERIOD=1 and its own private log path — not a whole-lane env flip, which races under -j (PipeStatsWindow.h:19-24 argues this).
  3. The lane has no positive control (ruling ID-115). Of the 7 entries that pass strict today, three are PushMonolithArm (where MGPipeServerStampVerbBoundary never runs, so CountBarrierPull is structurally unreachable), two self-skip, one is a death test and one is a Python check. Not one is a record-carrying split GL scenario. So today's "green" is indistinguishable from "strict never armed". Add an assertion that at least one drawing inproc entry completed with a server-stamped verb boundary. Without it, 116/116 means nothing.

Item 7 — partition the lane by backend, or admit Magma's row

Both DirectVulkan.Split.NamedBlit.* entries carry LABELS "integration-split" (MG_IntegrationTest/CMakeLists.txt:2270-2280) and abort on GetFramebufferBindingSlot@Clear (VulkanRenderer.cpp:7648), which P5e does not unbarrier — GetFramebufferBindingSlot retires in P7 on Magma. CONTRACT-P5E §7 already says in prose that Magma keeps the expected-red step. Give those entries an integration-split-magma label with their own expected-red step, so the Espryt allowlist stays a statement about Espryt. If you find a reason to prefer admitting the row instead, argue it.

Item 8 — mark contract amendment 8 UNLANDED (ruling ID-120)

CONTRACT-P5E.md:503-506 says GetBufferBindingPointCount's sticky forward becomes FATAL under split. FieldOwnership.def:147 and :179 both still say BARRIER_PULLED, and the generator refuses a disagreeing field/forward pair (pinned by FieldOwnershipTest.TheSevenStickyForwardsAgreeWithTheirFieldRows). The amendment is unlanded and landing it as written would change 10 pairs of the derived set. Do not land it. Annotate it in the contract as UNLANDED with the phase that will land it (P7/P13), so the divergence is stated rather than discovered.


Gate before you report

build rc=0 · unit 2250/2250 (expect this number to move — say by how much and why) · isplit 116/116 · gpu 1294/1294 · gens all rc=0, including your new generator arm and its self-test · and the strict lane, which after your change should complete every entry. Report the full marker table split into Fatal{ (must be only what pa and mv still owe) and Admitted{.

Expect the marker table to shift, not vanish. The census is a first-fault histogram: entries that abort today on GetProgramForDraw@DrawArrays will, once that row lands, advance to their next pull. Do not read a changed table as a regression without checking which fault moved.

Report

~/w7/notes/p5e/reports/gl-v1.md: per item, what you did, the red-once verbatim, and for any item you did not reach, exactly where you stopped and why. Also state plainly whether, with your changes, "the strict lane is hard green" is now a statement a script can evaluate — and if not, what is still missing.