commit 3fef2abb8fbeb4d5b183b3807140a1dd703b74ab
author Corentin Wallez <cwallez@chromium.org> Tue Feb 25 14:16:46 2025 -0800
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Feb 25 14:16:46 2025 -0800
tree 5378ae1f7c1238ad5ebb17a94318061c949c0c1c
parent 8bdc3b9c5a08a2c18a96e49b10cf3dbd4a1a0ea5

[dawn][d3d12] Remove redundant wait on SRB EndAccess

The common code from SharedResourceMemory already takes care of
propagating the BeginAccess fences if the resource wasn't used.

This was found when auditing the uses of EnsureCommandsFlushed.

Bug: 42241325, 42242066
Change-Id: I9a499b9a10597ce2e5453790f426cb621eb40b2d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/227294
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Brandon1 Jones <brandon1.jones@intel.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn/native/SharedResourceMemory.h

diff --git a/src/dawn/native/SharedResourceMemory.h b/src/dawn/native/SharedResourceMemory.h
index 587cbae..c552299 100644
--- a/src/dawn/native/SharedResourceMemory.h
+++ b/src/dawn/native/SharedResourceMemory.h
@@ -44,6 +44,9 @@
 
 enum SharedResourceAccessState { NotAccessed, ExclusiveRead, SimultaneousRead, Write };
 
+// The interface for the resources that can be created from a SharedResourceMemory (SRM).
+// Note that in practice this is only Buffer and Texture and that some internals of SRM assume that
+// with static polymorphism.
 class SharedResource : public ApiObjectBase {
   public:
     using ApiObjectBase::ApiObjectBase;
@@ -97,9 +100,6 @@
 
     SharedResourceMemoryContents* GetContents() const;
 
-    // Validate that the resource was created from this SharedResourceMemory.
-    MaybeError ValidateResourceCreatedFromSelf(SharedResource* resource);
-
   protected:
     SharedResourceMemory(DeviceBase* device, ObjectBase::ErrorTag, StringView label);
     using ApiObjectBase::ApiObjectBase;
@@ -107,6 +107,9 @@
   private:
     virtual Ref<SharedResourceMemoryContents> CreateContents();
 
+    // Validate that the resource was created from this SharedResourceMemory.
+    MaybeError ValidateResourceCreatedFromSelf(SharedResource* resource);
+
     template <typename Resource, typename BeginAccessDescriptor>
     MaybeError BeginAccess(Resource* resource, const BeginAccessDescriptor* rawDescriptor);
 
@@ -138,6 +141,7 @@
         ExecutionSerial lastUsageSerial,
         UnpackedPtr<SharedBufferMemoryEndAccessState>& state);
 
+    // If non-null, the SRM is exclusively accessed from that SR, used for validation.
     Ref<SharedResource> mExclusiveAccess;
     Ref<SharedResourceMemoryContents> mContents;
 };
@@ -163,11 +167,14 @@
   private:
     friend class SharedResourceMemory;
 
+    // The fences that must be waited on before the next use of the resource, whether that use is
+    // internal to Dawn or external (when exporting on EndAccess).
     PendingFenceList mPendingFences;
 
     SharedResourceAccessState mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
     int mReadAccessCount = 0;
 
+    // A pointer to the parent SRM that's a weak to prevent potential ref cycles.
     WeakRef<SharedResourceMemory> mSharedResourceMemory;
 };
 

src/dawn/native/d3d12/BufferD3D12.cpp

diff --git a/src/dawn/native/d3d12/BufferD3D12.cpp b/src/dawn/native/d3d12/BufferD3D12.cpp
index ddcc48a..b4fec84 100644
--- a/src/dawn/native/d3d12/BufferD3D12.cpp
+++ b/src/dawn/native/d3d12/BufferD3D12.cpp
@@ -629,21 +629,6 @@
     return {};
 }
 
-MaybeError Buffer::EndAccess() {
-    Queue* queue = ToBackend(GetDevice()->GetQueue());
-    // Synchronize here if the shared buffer hasn't already been synchronized due to a previous
-    // access.
-    if (mLastUsageSerial == kBeginningOfGPUTime) {
-        DAWN_TRY(SynchronizeBufferBeforeUse());
-        DAWN_ASSERT(mLastUsageSerial != kBeginningOfGPUTime);
-    }
-    // Make the queue signal the fence in finite time.
-    DAWN_TRY(queue->EnsureCommandsFlushed(mLastUsageSerial));
-
-    mLastUsageSerial = kBeginningOfGPUTime;
-    return {};
-}
-
 MaybeError Buffer::SynchronizeBufferBeforeUse() {
     // Buffers imported with the SharedBufferMemory feature can include fences that must finish
     // before Dawn can use the buffer. We acquire and wait for them here.

src/dawn/native/d3d12/BufferD3D12.h

diff --git a/src/dawn/native/d3d12/BufferD3D12.h b/src/dawn/native/d3d12/BufferD3D12.h
index 3a9a4ae..e73840e 100644
--- a/src/dawn/native/d3d12/BufferD3D12.h
+++ b/src/dawn/native/d3d12/BufferD3D12.h
@@ -71,7 +71,6 @@
     MaybeError EnsureDataInitializedAsDestination(CommandRecordingContext* commandContext,
                                                   const CopyTextureToBufferCmd* copy);
 
-    MaybeError EndAccess();
     MaybeError SynchronizeBufferBeforeUse();
 
     // Dawn API

src/dawn/native/d3d12/SharedBufferMemoryD3D12.cpp

diff --git a/src/dawn/native/d3d12/SharedBufferMemoryD3D12.cpp b/src/dawn/native/d3d12/SharedBufferMemoryD3D12.cpp
index 005a791..4dca66d 100644
--- a/src/dawn/native/d3d12/SharedBufferMemoryD3D12.cpp
+++ b/src/dawn/native/d3d12/SharedBufferMemoryD3D12.cpp
@@ -151,8 +151,6 @@
                     "Required feature (%s) is missing.",
                     wgpu::FeatureName::SharedFenceDXGISharedHandle);
 
-    DAWN_TRY(ToBackend(buffer)->EndAccess());
-
     Ref<d3d::SharedFence> sharedFence;
     DAWN_TRY_ASSIGN(sharedFence, ToBackend(GetDevice()->GetQueue())->GetOrCreateSharedFence());