swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-11-07 01:53:24 +09:00
Code Issues Packages Projects Releases Wiki Activity

[MG_Backend, MG_Test] (Disaggregated): P5e ID-117 - the strict knob gains an admitted state

Browse Source 
- CountBarrierPull aborted under MOBILEGL_IPC_STRICT_ERRORS on EVERY barrier-pulled read,
  admitted or not. The run's rc is therefore non-zero before CI ever reaches its allowlist
  comparison, so that comparison was unreachable code and "hard green" was impossible with the
  knob as written - the lane could not tell a debt this phase leaves standing from a defect.
- Third state, with the other two untouched: unbarriered stays an unconditional Fatal; barriered
  and NOT admitted stays a Fatal under strict; barriered and admitted logs one MGLOG_W and the
  entry completes. Admission is MGPipeBarrierPullAdmitted, generated from the three tables
  (ID-116), so there is no list here to drift.
- The marker keeps the grammar and changes only the tag: Admitted{UnmigratedPipeInput,
  "<F>@<V>"} [BARRIER-PULLED, ADMITTED, retires in <phase>]. Every filter written since P5c
  matches Fatal{UnmigratedPipeInput and therefore still means exactly "red".
- Deduped per (field, verb) per process. An 852-draw frame reaches an admitted readback row once
  per draw; undeduped, one frame writes 852 identical lines and the marker census is unreadable.
- CHECKED, NOT ASSUMED (the brief's instruction): StrictErrorsTurnsABarrierPulledReadIntoANamedAbort
  and StrictErrorsAlsoPromotesTheStickyForwards both still go red. Both stamp DrawArrays, whose
  op is kWaitNone, so neither pair is admitted.
- ID-119: the old "rsp = 0 on unbarriered records" comment at this function is withdrawn as
  vacuous - the unbarriered arm is [[noreturn]] and runs BEFORE ++g_residualPulls, so the
  sentence was true of every possible implementation.
- Red-once: AnAdmittedBarrierPullIsLoudOnceAndNotFatal is the same sticky forward as the case
  above it under a barriered verb; making the strict arm unconditional again kills it -
  "FieldOwnershipTest.cpp:867: Failure / Actual: false / signal 6".
This commit is contained in:
swung0x48
2026-09-18 11:58:19 -04:00
parent a211756aec
commit 4f492edbf3
2 changed files with 110 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/MGPipe/PipeInputs.cpp
+63 -3
View File
@@ -74,6 +74,40 @@ namespace MobileGL::MG_Pipe {
std::abort();
}
// P5e (gl), ID-117: THE ADMITTED MARKER, AND WHY IT IS ONLY THE TAG THAT CHANGED.
//
// `<field>@<verb>` and the `[BARRIER-PULLED, …, retires in <phase>]` tail are IDENTICAL
// to the Fatal above; the leading tag is `Admitted{` instead of `Fatal{`. Every filter
// written since P5c matches `Fatal{UnmigratedPipeInput`, so all of them keep meaning
// exactly "red" and none of them has to learn a new grammar to keep meaning it. A reader
// grepping for the pair still finds it, and the lane can ratchet on both sets with one
// regex per tag (ID-119's two-sided ratchet).
//
// THE DEDUPE IS NOT TIDINESS. An 852-draw Minecraft frame reaches an admitted readback
// row once per draw; without this, one frame writes 852 identical lines, the log file is
// the size of the run and the marker census cannot be read at all. So it is once per
// (field, verb) per process, which is the granularity the allowlist is written in.
//
// A PLAIN ARRAY, NOT AN ATOMIC, and that is the file's existing rule rather than a
// shortcut: CountBarrierPull is reachable only after the server stamped a verb boundary,
// which happens on the apply thread inside PipeApplier::ApplyOne, and g_residualPulls
// beside it is a plain Uint64 for the same reason. A racing writer here would at worst
// log a duplicate line, never lose one.
void AdmittedBarrierPullOnce(MGPipeInputField field, MGPipeVerb verb) {
const SizeT fieldIndex = static_cast<SizeT>(field);
const SizeT verbIndex = static_cast<SizeT>(verb);
if (fieldIndex >= kMGPipeInputFieldCount || verbIndex >= kMGPipeVerbCount) return;
static Uint64 seen[(kMGPipeVerbCount * kMGPipeInputFieldCount + 63) / 64] = {};
const SizeT bit = verbIndex * kMGPipeInputFieldCount + fieldIndex;
const Uint64 mask = Uint64{1} << (bit % 64);
if ((seen[bit / 64] & mask) != 0) return;
seen[bit / 64] |= mask;
MGLOG_W("MGPipe: Admitted{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, ADMITTED, "
"retires in %s]",
kMGPipeInputFieldNames[fieldIndex], MGPipeVerbName(verb),
kMGPipeFieldRetiringPhase[fieldIndex]);
}
// One place decides what a BARRIER-PULLED read does, so the field accessors and the
// seven sticky forwards cannot drift apart on it.
//
@@ -89,8 +123,31 @@ namespace MobileGL::MG_Pipe {
// torn or stale BY CONSTRUCTION and there is nothing for a counter to count. A
// "count it" arm here would be a wrong picture with a number beside it.
//
// rsp is therefore 0 on unbarriered records in every scenario summary (§7's pin): a
// non-zero count would be an abort that did not fire.
// P5e (gl), ID-119: THE OLD PIN HERE - "rsp is 0 on unbarriered records" - WAS VACUOUS
// AND HAS BEEN WITHDRAWN. The unbarriered arm below is [[noreturn]] and runs BEFORE
// ++g_residualPulls, so an unbarriered pull can never reach the counter whatever this
// function is written to do; the sentence was true of every possible implementation and
// therefore checked nothing. What rsp actually counts is BARRIERED pulls, and under the
// strict knob it counts exactly the ADMITTED ones, because every other barriered pull
// aborts two lines down. That is the form CONTRACT-P5E §7 now states and the lane checks.
//
// ---- P5e (gl), ID-117: AN ADMITTED PULL IS LOUD, NOT FATAL ---------------------------
//
// Before this, strict aborted on EVERY barrier-pulled read, admitted or not - which made
// the CI's allowlist comparison unreachable code (the run's rc != 0 exits first) and made
// "hard green" impossible with the knob as written. So there is a third state, and the
// two existing ones are untouched:
//
// unbarriered -> Fatal, no knob. The value is torn by construction.
// barriered, unadmitted -> Fatal under strict. A debt no phase has taken.
// barriered, admitted -> ONE MGLOG_W per (field, verb), and the entry completes.
//
// An admitted pull is a debt this phase deliberately leaves standing: the field's row is
// BARRIER_PULLED, the verb's op is statically barriered so the client really is parked
// behind the record, and the field is inside the verb's own may-read mask - so the value
// is real, ordered and fresh, exactly ruling 4's "barriered records keep P5C semantics".
// MGPipeBarrierPullAdmitted is generated from those three tables (ID-116); there is no
// list here to drift.
void CountBarrierPull(MGPipeInputField field, MGPipeVerb verb) {
if (!MGPipeApplierCurrentRecordIsBarriered()) {
StrictBarrierPullFatal(field, verb, "UNBARRIERED, the client did not fill it");
@@ -100,7 +157,10 @@ namespace MobileGL::MG_Pipe {
MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::ResidualPulls, 1);
}
if (MG_Config::Ipc.StrictErrors) {
StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1");
if (!MGPipeBarrierPullAdmitted(field, verb)) {
StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1");
}
AdmittedBarrierPullOnce(field, verb);
}
}
MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp
+47
View File
@@ -841,6 +841,53 @@ TEST_F(FieldOwnershipTest, StrictErrorsAlsoPromotesTheStickyForwards) {
<< r.Log;
}
// P5e (gl) RED-ONCE, ID-117: THE THIRD STATE, AND IT IS THE SAME FIELD AS THE CASE ABOVE.
//
// ValidateProgramName@DrawArrays aborts (draw_vbo is kWaitNone: the client will not be parked
// behind that record and the read is torn by construction). ValidateProgramName@ReadPixels is
// the same sticky forward under a verb whose op IS statically barriered, so the client is
// parked, the value is real and ordered, and the debt is P9's rather than this phase's - it
// says so and the entry COMPLETES. One field, two verbs, opposite verdicts: that is the whole
// of ID-84 in two cases, and before this change the lane could not tell them apart because
// strict aborted on both, which made CI's allowlist comparison unreachable code.
//
// THE RED: delete the `if (!MGPipeBarrierPullAdmitted(...))` guard in CountBarrierPull so the
// strict arm is unconditional again, and this case dies of SIGABRT instead of exiting 0.
TEST_F(FieldOwnershipTest, AnAdmittedBarrierPullIsLoudOnceAndNotFatal) {
const ChildResult r = RunInChild([] {
MG_Config::Ipc.StrictErrors = true;
MGPipeServerStampVerbBoundary(MGPipeVerb::ReadPixels);
(void)gPipeInputs.ValidateProgramName(1u);
(void)gPipeInputs.ValidateProgramName(1u);
(void)gPipeInputs.ValidateProgramName(2u);
// rsp counts every barriered pull, and under strict every surviving barriered pull is an
// admitted one - ID-119's reworded pin, from the counter's side.
if (MGPipeResidualPullCount() != 3) ::_exit(7);
});
ASSERT_TRUE(ExitedWith(r, 0)) << DescribeStatus(r) << "\n" << r.Log;
EXPECT_EQ(r.Log.find("Fatal{UnmigratedPipeInput"), std::string::npos)
<< "an admitted pull aborted: the lane can never be green while a debt this phase "
"deliberately leaves standing is fatal\n"
<< r.Log;
// THE GRAMMAR, VERBATIM. `<field>@<verb>` and the bracketed tail are the Fatal's, character
// for character; only the leading tag differs, so every filter written since P5c that
// matches `Fatal{UnmigratedPipeInput` still means exactly "red".
EXPECT_NE(r.Log.find("MGPipe: Admitted{UnmigratedPipeInput, \"ValidateProgramName@ReadPixels\"}"
" [BARRIER-PULLED, ADMITTED, retires in P9]"),
std::string::npos)
<< r.Log;
// DEDUPED PER (field, verb): three pulls, one line. An 852-draw frame must not write 852.
SizeT lines = 0;
for (std::size_t at = r.Log.find("Admitted{UnmigratedPipeInput"); at != std::string::npos;
at = r.Log.find("Admitted{UnmigratedPipeInput", at + 1)) {
++lines;
}
EXPECT_EQ(lines, SizeT{1})
<< "the admitted marker is not deduped per (field, verb); a frame of draws would write "
"one line per pull and the marker census could not be read\n"
<< r.Log;
}
// A FATAL-class read aborts whatever the knob says: no carrier, and the reduced path never
// reads it, so it is a real defect rather than a debt.
// P5b i1: the exemplar MOVED. This case used GetProgramForDispatch, which is BARRIER-PULLED