Make d3d12 concurrent read assertion more precise
Previously, IsReadOnly did not distinguish concurrent read,
and exclusive read. For STM textures, they must either be valid
to decay for concurrent read, or they must have exclusive read or
write access.
To resolve investigation in:
https://chromium-review.googlesource.com/c/chromium/src/+/5456006/comments/6be39e00_43ea73e0
Change-Id: I61f162a0cd44012fb9dfb1ae216e19f93b1d125b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/185723
Reviewed-by: Jie A Chen <jie.a.chen@intel.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn/native/SharedResourceMemory.cpp b/src/dawn/native/SharedResourceMemory.cpp
index 334c792..296e993 100644
--- a/src/dawn/native/SharedResourceMemory.cpp
+++ b/src/dawn/native/SharedResourceMemory.cpp
@@ -53,15 +53,15 @@
void SharedResourceMemory::DestroyImpl() {}
-bool SharedResourceMemory::HasWriteAccess() const {
+bool SharedResourceMemoryContents::HasWriteAccess() const {
return mSharedResourceAccessState == SharedResourceAccessState::Write;
}
-bool SharedResourceMemory::HasExclusiveReadAccess() const {
+bool SharedResourceMemoryContents::HasExclusiveReadAccess() const {
return mSharedResourceAccessState == SharedResourceAccessState::ExclusiveRead;
}
-int SharedResourceMemory::GetReadAccessCount() const {
+int SharedResourceMemoryContents::GetReadAccessCount() const {
return mReadAccessCount;
}
@@ -126,39 +126,39 @@
"%s with multiplanar format (%s) must be initialized.", resource,
resource->GetFormat().format);
- DAWN_INVALID_IF(HasWriteAccess(), "%s is currently accessed for writing.", this);
- DAWN_INVALID_IF(HasExclusiveReadAccess(), "%s is currently accessed for exclusive reading.",
- this);
+ DAWN_INVALID_IF(mContents->HasWriteAccess(), "%s is currently accessed for writing.", this);
+ DAWN_INVALID_IF(mContents->HasExclusiveReadAccess(),
+ "%s is currently accessed for exclusive reading.", this);
if (static_cast<TextureBase*>(resource)->IsReadOnly()) {
if (descriptor->concurrentRead) {
DAWN_ASSERT(!mExclusiveAccess);
DAWN_INVALID_IF(!descriptor->initialized, "Concurrent reading an uninitialized %s.",
resource);
- ++mReadAccessCount;
- mSharedResourceAccessState = SharedResourceAccessState::SimultaneousRead;
+ ++mContents->mReadAccessCount;
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::SimultaneousRead;
} else {
DAWN_INVALID_IF(
- mReadAccessCount != 0,
+ mContents->mReadAccessCount != 0,
"Exclusive read access used while %s is currently accessed for reading.", this);
- mSharedResourceAccessState = SharedResourceAccessState::ExclusiveRead;
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::ExclusiveRead;
mExclusiveAccess = resource;
}
} else {
DAWN_INVALID_IF(descriptor->concurrentRead, "Concurrent reading read-write %s.",
resource);
- DAWN_INVALID_IF(mReadAccessCount != 0,
+ DAWN_INVALID_IF(mContents->mReadAccessCount != 0,
"Read-Write access used while %s is currently accessed for reading.",
this);
- mSharedResourceAccessState = SharedResourceAccessState::Write;
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::Write;
mExclusiveAccess = resource;
}
} else if constexpr (std::is_same_v<Resource, BufferBase>) {
DAWN_INVALID_IF(mExclusiveAccess != nullptr,
"Cannot begin access with %s on %s which is currently accessed by %s.",
resource, this, mExclusiveAccess.Get());
- mSharedResourceAccessState = SharedResourceAccessState::Write;
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::Write;
mExclusiveAccess = resource;
}
@@ -229,24 +229,24 @@
DAWN_INVALID_IF(!resource->HasAccess(), "%s is not currently being accessed.", resource);
if constexpr (std::is_same_v<Resource, TextureBase>) {
if (static_cast<TextureBase*>(resource)->IsReadOnly()) {
- DAWN_ASSERT(!HasWriteAccess());
- if (HasExclusiveReadAccess()) {
- DAWN_ASSERT(mReadAccessCount == 0);
- mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
+ DAWN_ASSERT(!mContents->HasWriteAccess());
+ if (mContents->HasExclusiveReadAccess()) {
+ DAWN_ASSERT(mContents->mReadAccessCount == 0);
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
mExclusiveAccess = nullptr;
} else {
- DAWN_ASSERT(mSharedResourceAccessState ==
+ DAWN_ASSERT(mContents->mSharedResourceAccessState ==
SharedResourceAccessState::SimultaneousRead);
DAWN_ASSERT(mExclusiveAccess == nullptr);
- --mReadAccessCount;
- if (mReadAccessCount == 0) {
- mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
+ --mContents->mReadAccessCount;
+ if (mContents->mReadAccessCount == 0) {
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
}
}
} else {
- DAWN_ASSERT(mSharedResourceAccessState == SharedResourceAccessState::Write);
- DAWN_ASSERT(mReadAccessCount == 0);
- mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
+ DAWN_ASSERT(mContents->mSharedResourceAccessState == SharedResourceAccessState::Write);
+ DAWN_ASSERT(mContents->mReadAccessCount == 0);
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
mExclusiveAccess = nullptr;
}
} else if constexpr (std::is_same_v<Resource, BufferBase>) {
@@ -256,7 +256,7 @@
DAWN_INVALID_IF(mExclusiveAccess != resource,
"Cannot end access with %s on %s which is currently accessed by %s.",
resource, this, mExclusiveAccess.Get());
- mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
+ mContents->mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
mExclusiveAccess = nullptr;
}
diff --git a/src/dawn/native/SharedResourceMemory.h b/src/dawn/native/SharedResourceMemory.h
index 64ec58e..632b98f 100644
--- a/src/dawn/native/SharedResourceMemory.h
+++ b/src/dawn/native/SharedResourceMemory.h
@@ -100,10 +100,6 @@
SharedResourceMemory(DeviceBase* device, ObjectBase::ErrorTag, const char* label);
using ApiObjectBase::ApiObjectBase;
- bool HasWriteAccess() const;
- bool HasExclusiveReadAccess() const;
- int GetReadAccessCount() const;
-
private:
virtual Ref<SharedResourceMemoryContents> CreateContents();
@@ -136,8 +132,6 @@
UnpackedPtr<SharedBufferMemoryEndAccessState>& state);
Ref<SharedResource> mExclusiveAccess;
- SharedResourceAccessState mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
- int mReadAccessCount = 0;
Ref<SharedResourceMemoryContents> mContents;
};
@@ -160,12 +154,19 @@
const WeakRef<SharedResourceMemory>& GetSharedResourceMemory() const;
+ bool HasWriteAccess() const;
+ bool HasExclusiveReadAccess() const;
+ int GetReadAccessCount() const;
+
private:
friend class SharedResourceMemory;
PendingFenceList mPendingFences;
ExecutionSerial mLastUsageSerial{0};
+ SharedResourceAccessState mSharedResourceAccessState = SharedResourceAccessState::NotAccessed;
+ int mReadAccessCount = 0;
+
WeakRef<SharedResourceMemory> mSharedResourceMemory;
};
diff --git a/src/dawn/native/d3d12/TextureD3D12.cpp b/src/dawn/native/d3d12/TextureD3D12.cpp
index 39d01f2..8764b94 100644
--- a/src/dawn/native/d3d12/TextureD3D12.cpp
+++ b/src/dawn/native/d3d12/TextureD3D12.cpp
@@ -589,8 +589,8 @@
// state at all times that read accesses are happening; otherwise, the
// texture can enter a state where it could be modified by one read access
// (e.g., to be compressed or decrompessed) while being read by another.
- bool inReadAccess = HasAccess() && IsReadOnly();
- DAWN_ASSERT(state->isValidToDecay || !inReadAccess);
+ DAWN_ASSERT(state->isValidToDecay || mSharedResourceMemoryContents->HasWriteAccess() ||
+ mSharedResourceMemoryContents->HasExclusiveReadAccess());
}
D3D12_RESOURCE_BARRIER barrier;
