Rename BufferState::InsideOperation to InUse This is to reflect the state is set more broadly by queue or device operations. Bug: 425472913 Change-Id: I14372b0de0348a84c48439896ce9ae1d5974cce1 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/286375 Reviewed-by: Loko Kung <lokokung@google.com> Commit-Queue: Kyle Charbonneau <kylechar@google.com>
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp index efd735c..69e619e 100644 --- a/src/dawn/native/Buffer.cpp +++ b/src/dawn/native/Buffer.cpp
@@ -477,12 +477,12 @@ state = mState.load(std::memory_order::acquire); break; } - case BufferState::InsideOperation: { + case BufferState::InUse: { // This is never supposed to happen but another operation is happening concurrently // with API Destroy() call. [[maybe_unused]] bool hadError = GetDevice()->ConsumedError(ConcurrentUseError(), "calling %s.Destroy().", this); - while (mState.load(std::memory_order::acquire) == BufferState::InsideOperation) { + while (mState.load(std::memory_order::acquire) == BufferState::InUse) { // Spin loop instead of wait() to avoid overhead of signal in map/unmap. } break; @@ -543,7 +543,7 @@ DAWN_ASSERT(!mIsHostMapped); ABSL_FALLTHROUGH_INTENDED; case BufferState::Destroyed: - case BufferState::InsideOperation: + case BufferState::InUse: case BufferState::SharedMemoryNoAccess: return wgpu::BufferMapState::Unmapped; } @@ -669,7 +669,7 @@ return DAWN_VALIDATION_ERROR("%s used in submit while pending map.", this); case BufferState::SharedMemoryNoAccess: return DAWN_VALIDATION_ERROR("%s used in submit without shared memory access.", this); - case BufferState::InsideOperation: + case BufferState::InUse: return ConcurrentUseError(); case BufferState::Unmapped: return {}; @@ -706,7 +706,7 @@ case BufferState::Mapped: case BufferState::MappedAtCreation: return DAWN_VALIDATION_ERROR("%s is already mapped.", this); - case BufferState::InsideOperation: + case BufferState::InUse: return ConcurrentUseError(); case BufferState::PendingMap: return DAWN_VALIDATION_ERROR("%s already has an outstanding map pending.", @@ -719,7 +719,7 @@ break; } - DAWN_TRY(TransitionState(BufferState::Unmapped, BufferState::InsideOperation)); + DAWN_TRY(TransitionState(BufferState::Unmapped, BufferState::InUse)); DAWN_TRY_WITH_CLEANUP(MapAsyncImpl(mode, offset, size), { // Reset state since an error stopped this from reaching pending map state. mState.store(BufferState::Unmapped, std::memory_order::release); @@ -831,12 +831,12 @@ MaybeError BufferBase::Unmap(bool forDestroy) { switch (mState.load(std::memory_order::acquire)) { case BufferState::Mapped: - DAWN_TRY(TransitionState(BufferState::Mapped, BufferState::InsideOperation)); + DAWN_TRY(TransitionState(BufferState::Mapped, BufferState::InUse)); UnmapImpl(BufferState::Mapped, forDestroy ? BufferState::Destroyed : BufferState::Unmapped); break; case BufferState::MappedAtCreation: - DAWN_TRY(TransitionState(BufferState::MappedAtCreation, BufferState::InsideOperation)); + DAWN_TRY(TransitionState(BufferState::MappedAtCreation, BufferState::InUse)); if (mStagingBuffer != nullptr) { if (forDestroy) { // No need to upload staging contents if the buffer is being destroyed. @@ -852,7 +852,7 @@ forDestroy ? BufferState::Destroyed : BufferState::Unmapped); } break; - case BufferState::InsideOperation: + case BufferState::InUse: return ConcurrentUseError(); case BufferState::Unmapped: return {}; @@ -892,7 +892,7 @@ : "Buffer was unmapped before mapping was resolved."); BufferState exchangedState = - mState.exchange(BufferState::InsideOperation, std::memory_order::acq_rel); + mState.exchange(BufferState::InUse, std::memory_order::acq_rel); DAWN_CHECK(exchangedState == BufferState::PendingMap); } } @@ -976,7 +976,7 @@ case BufferState::PendingMap: case BufferState::Unmapped: - case BufferState::InsideOperation: + case BufferState::InUse: case BufferState::SharedMemoryNoAccess: case BufferState::Destroyed: return false;
diff --git a/src/dawn/native/Buffer.h b/src/dawn/native/Buffer.h index 55f6b83..863cc7f 100644 --- a/src/dawn/native/Buffer.h +++ b/src/dawn/native/Buffer.h
@@ -85,8 +85,8 @@ enum class BufferState { // MapAsync() or Unmap() is in progress. // TODO(crbug.com/467247254): See if ConcurrentAccessGuard<T> can be used be implemented and - // used instead of having an InsideOperation state. - InsideOperation, + // used instead of having an InUse state. + InUse, Unmapped, PendingMap, Mapped, @@ -234,17 +234,17 @@ // guard against concurrent access to the buffer. // // The follow semantics are used: - // 1. On MapAsync() set state to InsideOperation before modifying any other members. If there is - // a race modifying the state compare_exchange() will fail and a validation error is thrown. - // After modifying all member variables set state to PendingMap. + // 1. On MapAsync() set state to InUse before modifying any other members. If there is a race + // modifying the state compare_exchange() will fail and a validation error is thrown. After + // modifying all member variables set state to PendingMap. // 2. When MapAsyncEvent completes set state to Mapped after all other work is finished. // 3. For *MappedRange() functions check that state is Mapped before checking other members for // validation. - // 4. For Unmap() set state to InsideOperation before modifying any other member variables. If - // there is a race modifying state compare_exchange() will fail and a validation error is - // thrown. After the buffer is unmapped set state to Unmapped. - // 5. For Destroy() check if the state is InsideOperation and if so spin loop until the - // concurrent operation is finished. This prevents destruction in the middle of an operation. + // 4. For Unmap() set state to InUse before modifying any other member variables. If there is a + // race modifying state compare_exchange() will fail and a validation error is thrown. After + // the buffer is unmapped set state to Unmapped. + // 5. For Destroy() check if the state is InUse and if so spin loop until the concurrent + // operation is finished. This prevents destruction in the middle of an operation. // // With those `mState` changes in place, we can guarantee that if GetMappedRange() is // successful, that MapAsync() must have succeeded. We cannot guarantee, however, that Unmap()