layers: Remove workaround for handling missing signals #8546
Conversation
|
CI Vulkan-ValidationLayers build queued with queue ID 257255. |
|
CI Vulkan-ValidationLayers build # 17476 running. |
| // In the correct program the resolving signal is the next signal on the timeline, | ||
| // otherwise this violates the rule of strictly increasing signal values. | ||
| const TimePoint *timepoint = nullptr; | ||
| for (auto it = timeline_.find(payload); it != timeline_.end(); ++it) { |
There was a problem hiding this comment.
Just realized this loop is basically what we had before, just with a placeholder to add error reporting later (probably not for the ongoing SDK). The main difference is removed workaround.
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
21cc26a
to
49069a5
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 257334. |
|
CI Vulkan-ValidationLayers build # 17477 running. |
|
Investigating crashes on CI machines. |
|
CI Vulkan-ValidationLayers build # 17477 aborted. |
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
49069a5
to
f70780a
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 257540. |
|
CI Vulkan-ValidationLayers build # 17479 running. |
|
CI Vulkan-ValidationLayers build # 17479 aborted. |
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
f70780a
to
b52427d
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 257596. |
|
CI Vulkan-ValidationLayers build # 17480 running. |
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
b52427d
to
289bd9c
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 257618. |
|
CI Vulkan-ValidationLayers build # 17481 running. |
|
CI Vulkan-ValidationLayers build # 17481 passed. |
|
The problem with CI crashes was that last change that should just cleanup things... |
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
289bd9c
to
0677251
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 257851. |
artem-lunarg
changed the title
|
CI Vulkan-ValidationLayers build # 17487 running. |
|
CI Vulkan-ValidationLayers build # 17487 passed. |
| @@ -1313,6 +1313,10 @@ TEST_F(PositiveSyncObject, FenceSemThreadRace) { | |||
| AddRequiredFeature(vkt::Feature::timelineSemaphore); | |||
| RETURN_IF_SKIP(Init()); | |||
|
|
|||
| if (IsPlatformMockICD()) { | |||
| GTEST_SKIP() << "Test not supported by MockICD (WaitSemaphores)"; | |||
There was a problem hiding this comment.
note: I have code in the CDL test framework that implements semaphores for the mock ICD. Could be a reason for us to re-converge them.
There was a problem hiding this comment.
Okay. Are you implementing mock synchronization for mock ICD? I was thinking about that, it would be really nice to remove this skips.
Signals registered in PreRecord are always available for non-external semaphores after waiting dependency is resolved. Also add placeholder and documentation for detecting non-increasing signal order.
artem-lunarg
force-pushed
the
artem-fix-semaphore-original-pre-record
branch
from
0677251
to
eb00bcb
Compare
|
CI Vulkan-ValidationLayers build queued with queue ID 258035. |
|
CI Vulkan-ValidationLayers build # 17492 running. |
|
CI Vulkan-ValidationLayers build # 17492 passed. |
Closes #8461
Discussed with
@jeremy-lunarg@jeremyg-lunarg reverted commit. PreRecord guarantees that any dependent PostRecord will see registered signals/waits in semaphore state object. There is a code that relies on this (when retiring non-external semaphore wait, there should be a signal).