We're supposed to completely ignore VkTimelineSemaphoreSubmitInfo if
there aren't any timeline semaphores, including the array lengths, which
is made clear by the various VUs already cited by the code. The
vkQueueSubmit() path correctly handled this when asserting but still
dereferenced pWaitSemaphoreValues unconditionally, which could lead to
dereferencing an invalid pointer if waitSemaphoreValueCount is less than
waitSemaphoreCount. The vkQueueSparseBind() path didn't even assert
correctly. Bring vkQueueSparseBind() in line with vkQueueSubmit()
and make both only dereference the wait/signal array once we've
determined it must be present. While we're here, also fix the assert in
vkQueueSubmit() to disallow a waitSemaphoreValueCount of 0 if there are
timeline semaphores present, which conversely is not allowed.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36989>
This gets rid of the internal wsi_memory_signal_submit_info structure
used to indicate implicit sync through vkQueueSubmit() as well as the
handling in vk_queue.c and vk_device::create_sync_for_memory. Nothing
is using any of this anymore.
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Acked-by: Daniel Stone <daniels@collabora.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36783>
In the C23 standard unreachable() is now a predefined function-like
macro in <stddef.h>
See https://android.googlesource.com/platform/bionic/+/HEAD/docs/c23.md#is-now-a-predefined-function_like-macro-in
And this causes build errors when building for C23:
-----------------------------------------------------------------------
In file included from ../src/util/log.h:30,
from ../src/util/log.c:30:
../src/util/macros.h:123:9: warning: "unreachable" redefined
123 | #define unreachable(str) \
| ^~~~~~~~~~~
In file included from ../src/util/macros.h:31:
/usr/lib/gcc/x86_64-linux-gnu/14/include/stddef.h:456:9: note: this is the location of the previous definition
456 | #define unreachable() (__builtin_unreachable ())
| ^~~~~~~~~~~
-----------------------------------------------------------------------
So don't redefine it with the same name, but use the name UNREACHABLE()
to also signify it's a macro.
Using a different name also makes sense because the behavior of the
macro was extending the one of __builtin_unreachable() anyway, and it
also had a different signature, accepting one argument, compared to the
standard unreachable() with no arguments.
This change improves the chances of building mesa with the C23 standard,
which for instance is the default in recent AOSP versions.
All the instances of the macro, including the definition, were updated
with the following command line:
git grep -l '[^_]unreachable(' -- "src/**" | sort | uniq | \
while read file; \
do \
sed -e 's/\([^_]\)unreachable(/\1UNREACHABLE(/g' -i "$file"; \
done && \
sed -e 's/#undef unreachable/#undef UNREACHABLE/g' -i src/intel/isl/isl_aux_info.c
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/36437>
It's still legal to include VkTimelineSemaphoreSubmitInfo in pNext
without any timeline semaphores. While we are at it, also fix the VUs.
This was observed with Doom The Dark Ages.
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/35022>
With vkCmdPipelineBarrier, it's possible to specify a barrier with
pipeline stages but without any memory barriers. These might not be
practical, but are legal Vulkan code.
Barriers like this are currently ignored in mesa, as we only convert
barriers with passed memory barriers into vkCmdPipelineBarrier2.
This commit adds handling of execution only barriers by converting them
into a memory barrier without access masks.
Fixes: 97f0a4494b ("vulkan: implement legacy entrypoints on top of VK_KHR_synchronization2")
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34187>
From the perspective of the gpu, host read or host write has the same
implication (gpu cache flush) in the dst access flags. We should
include host write in the dst access flags.
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32102>
VkAccessFlags2 and VkPipelineStageFlags2 being both 64-bit bitmasks
the mistake is harmless, but let's fix that anyway to avoid any
confusion.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Konstantin Seurer <konstantin.seurer@gmail.com>
Reviewed-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/30735>
STACK_ARRAY() is used in a lot of places. When games are running we
see STACK_ARRAY() arrays being used all the time: each queue
submission uses 6, WaitSemaphores and syncobj waiting also uses them:
they're constantly present in Vulkan runtime.
There's no need for STACK_ARRAY()'s stack array to be initialized,
callers cannot not depend on it. If the number of elements is greater
than STACK_ARRAY_SIZE, then STACK_ARRAY() will just malloc() the array
and return it not initialized: anybody depending of
zero-initialization is going to break when the array is big.
The reason why we're zero-intializing STACK_ARRAY()'s stack array is
to silence -Wmaybe-uninitialized warnings: see commit d7957df318
("vulkan: fix uninitialized variables"). I don't think that commit is
the ideal way to deal with the problem, so this patch proposes a
better solution.
The problem here is that zero-initializing it adds code we don't need
for every single caller. STACK_ARRAY() already has 63 callers and only
3 of them are affected by the -Wmaybe-uninitialized warining. So here
we undo what commit d7957df318 did and instead we fix the 3 cases
that actually generate the -Wmaybe-uninitialized warnings.
Gcc is only emitting those warinings because it knows that the number
of elements in the array may be zero, so the loops we have that set
elements to the array may end up do nothing, and then we pass the
array uninitialized to other functions.
For the cases related to vk_sync this is just returning VK_SUCCESS
earlier, instead of relying on the check that eventually happens at
__vk_sync_wait_many(). For the vkCmdWaitEvents() function, the Vulkan
spec says that "eventCount must be greater than 0", so the early
return doesn't hurt anybody either. In both cases we make the zero
case faster by not defining an 8-sized array, zero-initializing it,
then returning success without using it.
Reference: d7957df318 ("vulkan: fix uninitialized variables")
Acked-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Yiwei Zhang <zzyiwei@chromium.org>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28288>
This adds hand-written helpers for detecting if a set of stage flags
contains shaders as well as expand helpers that properly handle
TOP/BOTTOM_OF_PIPE_BIT.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Konstantin Seurer <konstantin.seurer@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26407>
I'm about to add more stuff that isn't really for sync2 so it makes
sense to give it a slightly more generic name.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Konstantin Seurer <konstantin.seurer@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26407>
2023-12-01 17:59:50 +00:00
Renamed from src/vulkan/runtime/vk_synchronization2.c (Browse further)