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_Pipe, MG_Test] (Disaggregated): P5e ID-118 - resource_copy_region becomes barriered

Browse Source 
- CopyImageSubData's apply reads GetTextureObject, a BARRIER_PULLED row that retires in P7, not
  in P5e. resource_copy_region was kWaitNone, so after the flip that is a genuinely UNBARRIERED
  record reading client memory: an unconditional Fatal, no knob, standing for two phases.
- The wait class moves rather than the field being special-cased into the allowlist. The
  allowlist is derived (ID-116), so an exception written into it would be a real defect kept
  quiet instead of a debt stated - and with the class moved the derivation admits the pair with
  no exception at all: 101 -> 111 pairs, CopyImageSubData joins the 12 barriered verbs.
- kWaitApplied AND NOT kWaitReply: the row posts no reply slot, so the client parks until the
  record applies. That is exactly what copy_framebuffer_to_texture - the two framebuffer-sourced
  copies, same kBlitOrCopy verb class - already does. THE COST IS MEASURED: glCopyImageSubData
  is zero per frame in the Minecraft 26.3-rc-3 scene, so the wait is on no per-frame path.
- PipeCatalogue's wait-class census moves with it, both halves in this commit: the named
  kWaitApplied set gains the row, the count goes 9 -> 10, and the kWaitNone offset 24 -> 25.
- Red-once: putting the row back to kWaitNone drops CopyImageSubData out of the derived
  allowlist (111 -> 101 pairs, 0 @CopyImageSubData) and fails
  "FieldOwnershipTest.cpp:422: Failure / resource_copy_region is unbarriered again: after the
  flip, a copy record reading the client's texture object is an unconditional Fatal and P7 is
  two phases away", with PipeCatalogueTest red on three counts beside it.
This commit is contained in:
swung0x48
2026-09-18 12:02:02 -04:00
parent 4f492edbf3
commit a6ddeb0816
5 changed files with 43 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Pipe/PipeCalls.def
+16 -1
View File
@@ -236,7 +236,22 @@
/* only way bytes reach it - a blobref here would be a SECOND way to say the same thing. */ \
X(ResourceFlushRange, MGPFlushRange, kCtxObject, kNone, kWaitNone) \
X(ResourceReadback, MGPReadback, kCtxObject, kReplySlot, kWaitReply) \
X(ResourceCopyRegion, MGPCopyRegion, kCtxObject, kNone, kWaitNone) \
/* P5e (gl), ID-118: kWaitNone -> kWaitApplied, AND IT IS THE FIELD'S RETIRING PHASE THAT */ \
/* forces it. This row's verb is CopyImageSubData, whose apply reads GetTextureObject - */ \
/* a BARRIER_PULLED row whose retiring phase is P7, not P5e. Left kWaitNone, the flip */ \
/* would make a genuinely UNBARRIERED record read client memory: an unconditional Fatal */ \
/* (PipeInputs.cpp), no knob, for two whole phases. The alternative was to special-case */ \
/* the field into section 7's allowlist, which ID-118 refused: the allowlist is DERIVED */ \
/* (ID-116), and an exception written into it would be a real defect kept quiet rather */ \
/* than a debt stated. With the wait class moved, the derivation admits the pair with no */ \
/* exception at all and P7 retires the field on its own schedule. */ \
/* kWaitApplied AND NOT kWaitReply: the call posts no reply slot and needs no answer - the */ \
/* client parks until the record applies, which is exactly what CopyFramebufferToTexture */ \
/* below already does for the two framebuffer-sourced copies. Same verb class */ \
/* (kBlitOrCopy), same shape, a wait that is already carried in this lane. THE COST IS */ \
/* MEASURED, NOT ASSUMED: glCopyImageSubData is zero per frame in the Minecraft 26.3-rc-3 */ \
/* scene (~849 draws/frame), so the wait is not on any per-frame path. */ \
X(ResourceCopyRegion, MGPCopyRegion, kCtxObject, kNone, kWaitApplied) \
X(GenerateMipmap, MGPMipPlan, kCtxObject, kNone, kWaitApplied) \
X(GetTextureImage, MGPReadbackInfo, kCtxObject, kReplySlot, kWaitReply) \
/* ---- context: transfer calls that read whole-context state, and the commands ---- */ \
MobileGL/MG_Pipe/generated/PipeFieldOwnership.inc
+2 -2
View File
@@ -356,7 +356,7 @@ inline constexpr MGPipeFieldMask kMGPipeAdmittedPullMask[kMGPipeVerbCount] = {
{{0x0000000000000000ull, 0x0000000000000000ull}}, // BlitNamedFramebuffer
{{0x5800c02000300180ull, 0x0000000000000000ull}}, // CopyTexImage2D
{{0x0000000000000000ull, 0x0000000000000000ull}}, // CopyTexSubImage2D
{{0x0000000000000000ull, 0x0000000000000000ull}}, // CopyImageSubData
{{0x5800c02000300180ull, 0x0000000000000000ull}}, // CopyImageSubData
{{0x5800c02000300180ull, 0x0000000000000000ull}}, // GenerateMipmap
{{0x5800c02000300140ull, 0x0000000000000000ull}}, // ReadPixels
{{0x0000000000000000ull, 0x0000000000000000ull}}, // GetTexImage
@@ -394,7 +394,7 @@ inline constexpr MGPipeFieldMask kMGPipeAdmittedPullMask[kMGPipeVerbCount] = {
{{0x0000000000000000ull, 0x0000000000000000ull}}, // GetGpuTimestampNs
};
inline constexpr SizeT kMGPipeAdmittedPullPairCount = 101;
inline constexpr SizeT kMGPipeAdmittedPullPairCount = 111;
// THE PREDICATE THE STRICT KNOB ASKS (ID-117). A 69 x 128-bit table, so this costs a shift and
// a test on the apply thread - the same shape MGPipeFieldMaskHas already has.
MobileGL/MG_Pipe/generated/PipeWire.inc
+1 -1
View File
@@ -428,7 +428,7 @@ inline constexpr Uint8 kMGPipeWaitClasses[static_cast<SizeT>(MGPWireOp::kOpCount
/* 50 ResourceSubDataComplete */ static_cast<Uint8>(kWaitNone),
/* 51 ResourceFlushRange */ static_cast<Uint8>(kWaitNone),
/* 52 ResourceReadback */ static_cast<Uint8>(kWaitReply),
/* 53 ResourceCopyRegion */ static_cast<Uint8>(kWaitNone),
/* 53 ResourceCopyRegion */ static_cast<Uint8>(kWaitApplied),
/* 54 GenerateMipmap */ static_cast<Uint8>(kWaitApplied),
/* 55 GetTextureImage */ static_cast<Uint8>(kWaitReply),
/* 56 Blit */ static_cast<Uint8>(kWaitNone),
MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp
+9
View File
@@ -413,6 +413,15 @@ TEST_F(FieldOwnershipTest, TheAdmittedPullTableIsID84sDerivationAndNotAList) {
"entries this phase is being run to fix";
EXPECT_FALSE(MGPipeBarrierPullAdmitted(MGPipeInputField::GetBoundVertexArray,
MGPipeVerb::DrawArrays));
// P5e (gl), ID-118: the pair that made resource_copy_region's wait class move. The field
// retires in P7, so it had to be admitted somewhere - and it is admitted HERE, by the
// derivation and with no exception, only because the op is kWaitApplied. Put that row back
// to kWaitNone in PipeCalls.def and this goes red, which is the whole argument for moving
// the wait class rather than writing the field into a list.
EXPECT_TRUE(MGPipeBarrierPullAdmitted(MGPipeInputField::GetTextureObject,
MGPipeVerb::CopyImageSubData))
<< "resource_copy_region is unbarriered again: after the flip, a copy record reading "
"the client's texture object is an unconditional Fatal and P7 is two phases away";
// A field that is not a debt at all is never admitted, on any verb.
EXPECT_FALSE(MGPipeBarrierPullAdmitted(MGPipeInputField::GetRenderStateParameters,
MGPipeVerb::ReadPixels));
MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp
+15 -4
View File
@@ -763,6 +763,15 @@ TEST(PipeCatalogue, EveryRowCarriesTheWaitClassTheContractGivesIt) {
MGPWireOp::EndStreamOutput, MGPWireOp::PauseStreamOutput,
MGPWireOp::ResumeStreamOutput, MGPWireOp::BindStreamOutput,
MGPWireOp::CopyFramebufferToTexture,
// P5e (gl), ID-118. resource_copy_region JOINED THIS SET, and it is the retiring phase
// of a field that put it here: its verb is CopyImageSubData, whose apply reads
// GetTextureObject, and that row retires in P7. An unbarriered record reading client
// memory is an unconditional Fatal, so leaving it kWaitNone would have meant a real
// defect standing for two phases - or an exception hand-written into a DERIVED
// allowlist, which is the same thing with a comment on it. It sits beside
// copy_framebuffer_to_texture here for the reason it took that class: same verb class,
// no reply slot, and zero calls per frame in the measured scene.
MGPWireOp::ResourceCopyRegion,
};
for (const MGPWireOp op : appliedRows) {
EXPECT_EQ(MGPipeWaitClassFor(op), kWaitApplied) << WireOpNameForDiag(op);
@@ -787,8 +796,10 @@ TEST(PipeCatalogue, EveryRowCarriesTheWaitClassTheContractGivesIt) {
EXPECT_EQ(MGPipeWaitClassFor(op), kWaitNone) << WireOpNameForDiag(op);
}
// The partition, by count. 14 + 1 + 9 = 24 rows wait; every other row of the catalogue does
// not. A row that changed class moves two of these numbers at once.
// The partition, by count. 14 + 1 + 10 = 25 rows wait; every other row of the catalogue does
// not. A row that changed class moves two of these numbers at once - which is why the
// kWaitApplied count went 9 -> 10 and the kWaitNone offset 24 -> 25 in the SAME commit that
// moved resource_copy_region (P5e gl, ID-118).
SizeT reply = 0, applied = 0, present = 0, none = 0, other = 0;
for (SizeT i = 1; i < static_cast<SizeT>(MGPWireOp::kOpCount); ++i) {
switch (MGPipeWaitClassFor(static_cast<MGPWireOp>(i))) {
@@ -800,9 +811,9 @@ TEST(PipeCatalogue, EveryRowCarriesTheWaitClassTheContractGivesIt) {
}
}
EXPECT_EQ(reply, 14u);
EXPECT_EQ(applied, 9u);
EXPECT_EQ(applied, 10u);
EXPECT_EQ(present, 1u);
EXPECT_EQ(none, static_cast<SizeT>(kMGPipeCallCount) - 24u);
EXPECT_EQ(none, static_cast<SizeT>(kMGPipeCallCount) - 25u);
EXPECT_EQ(other, 0u) << "a row carries the kWaitClassCount terminator as its class";
// And the reply half of the partition BOTH WAYS, over the whole catalogue: exactly the rows