vulkan: Detect pNext chain loops in vk_foreach_struct() (v2)

This implements the "tortoise and the hare" algorithm for detecting
cycles in graphs.  We use the caller's iterator as the hare and our own
internal copy as the tortoise.  Conveniently, VkBaseOutStructure (and
VkBaseInStructure which is identical except the pointer type on pNext)
have a pointer we can use for the tortoise and an sType which we can use
for a counter to ensure we only increment the tortose every other loop
iteration.

There are more efficient algorithms than tortoise and hare but they
require allocating memory for something like a hash set of seen nodes.
Since this for debug purposes only, it's ok for it to be a bit
inefficient in the case where it hits the assert.  In the usual case of
no loops, it's the same runtime efficiency as the unchecked version
except that it does a tiny bit of math and 50% more pointer chases.

Version 1 worked fine with clang and with GCC 12.1 with -O0 but not with
optimizations. See https://gitlab.freedesktop.org/mesa/mesa/-/issues/6895.
Unfortunately, the first version required modifying a temporary declared
const inside the for loop and that seems to have been the problem.  This
version instead has an iterator struct which is managed by an outer for
loop and the inner for loop exists to declare the user's requested
iteration variable and manage the actual iteration.  Because the outer
for loop is effectively `for (bool done = false; !done; done = true)`,
it will execute exactly once, regardless of the inner loop, so break and
continue inside the inner loop should work the same as if it's a single
for loop.

The other major difference with the new version is that the code is the
same for debug and release except the half_iter and loop check are gone.
I've verified by hand that this produces virtually identical code to the
old simple iterators on both GCC andl clang with an optimized build.

Reviewed-by: Jesse Natalie <jenatali@microsoft.com>
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17630>
This commit is contained in:
Jason Ekstrand 2022-07-18 09:05:59 -05:00 committed by Marge Bot
parent b510ee0d22
commit 3f19a60869
1 changed files with 70 additions and 6 deletions

View File

@ -37,13 +37,77 @@ extern "C" {
#include <vulkan/vulkan.h>
#define vk_foreach_struct(__iter, __start) \
for (struct VkBaseOutStructure *__iter = (struct VkBaseOutStructure *)(__start); \
__iter; __iter = __iter->pNext)
struct vk_pnext_iterator {
VkBaseOutStructure *pos;
#ifndef NDEBUG
VkBaseOutStructure *half_pos;
unsigned idx;
#endif
bool done;
};
static inline struct vk_pnext_iterator
vk_pnext_iterator_init(void *start)
{
struct vk_pnext_iterator iter;
iter.pos = (VkBaseOutStructure *)start;
#ifndef NDEBUG
iter.half_pos = (VkBaseOutStructure *)start;
iter.idx = 0;
#endif
iter.done = false;
return iter;
}
static inline struct vk_pnext_iterator
vk_pnext_iterator_init_const(const void *start)
{
return vk_pnext_iterator_init((void *)start);
}
static inline VkBaseOutStructure *
vk_pnext_iterator_next(struct vk_pnext_iterator *iter)
{
iter->pos = iter->pos->pNext;
#ifndef NDEBUG
if (iter->idx++ & 1) {
/** This the "tortoise and the hare" algorithm. We increment
* chaser->pNext every other time *iter gets incremented. Because *iter
* is incrementing twice as fast as chaser->pNext, the distance between
* them in the list increases by one for each time we get here. If we
* have a loop, eventually, both iterators will be inside the loop and
* this distance will be an integer multiple of the loop length, at
* which point the two pointers will be equal.
*/
iter->half_pos = iter->half_pos->pNext;
if (iter->half_pos == iter->pos)
assert(!"Vulkan input pNext chain has a loop!");
}
#endif
return iter->pos;
}
/* Because the outer loop only executes once, independently of what happens in
* the inner loop, breaks and continues should work exactly the same as if
* there were only one for loop.
*/
#define vk_foreach_struct(__e, __start) \
for (struct vk_pnext_iterator __iter = vk_pnext_iterator_init(__start); \
!__iter.done; __iter.done = true) \
for (VkBaseOutStructure *__e = __iter.pos; \
__e; __e = vk_pnext_iterator_next(&__iter))
#define vk_foreach_struct_const(__e, __start) \
for (struct vk_pnext_iterator __iter = \
vk_pnext_iterator_init_const(__start); \
!__iter.done; __iter.done = true) \
for (const VkBaseInStructure *__e = (VkBaseInStructure *)__iter.pos; \
__e; __e = (VkBaseInStructure *)vk_pnext_iterator_next(&__iter))
#define vk_foreach_struct_const(__iter, __start) \
for (const struct VkBaseInStructure *__iter = (const struct VkBaseInStructure *)(__start); \
__iter; __iter = __iter->pNext)
/**
* A wrapper for a Vulkan output array. A Vulkan output array is one that