<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Dec 7, 2018 at 6:06 PM Rafael Antognolli <<a href="mailto:rafael.antognolli@intel.com">rafael.antognolli@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">The test was checking whether -1 was smaller than an unsigned int, which<br>
is always false. So it was exiting early and never running until the<br>
end, since it would reach the condition (thread_max == -1).<br>
<br>
However, just fixing that is not enough. The test is currently getting<br>
the highest block on each iteration, and then on the next one, until it<br>
reaches the end. But by that point, we wouldn't have looked at all<br>
blocks of all threads. For instance, for 3 threads and 4 blocks per<br>
thread, a situation like this (unlikely to happen):<br>
<br>
[Thread]: [Blocks]<br>
[0]: [0, 32, 64, 96]<br>
[1]: [128, 160, 192, 224]<br>
[2]: [256, 288, 320, 352]<br>
<br>
Would cause the test to iterate only over the thread number 2.<br>
<br>
The fix is to always grab the lowest block on each iteration, and assert<br>
that it is higher than the one from the previous iteration.<br>
---<br>
src/intel/vulkan/tests/block_pool_no_free.c | 17 +++++++++--------<br>
1 file changed, 9 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/src/intel/vulkan/tests/block_pool_no_free.c b/src/intel/vulkan/tests/block_pool_no_free.c<br>
index 17006dd3bc7..730297d4e36 100644<br>
--- a/src/intel/vulkan/tests/block_pool_no_free.c<br>
+++ b/src/intel/vulkan/tests/block_pool_no_free.c<br>
@@ -78,16 +78,16 @@ static void validate_monotonic(uint32_t **blocks)<br>
unsigned next[NUM_THREADS];<br>
memset(next, 0, sizeof(next));<br>
<br>
- int highest = -1;<br>
+ uint32_t lowest = UINT32_MAX;<br></blockquote><div><br></div><div>I think this should still be named "highest" since it is the highest thing we've seen. Also, I kind-of think -1 still makes sense as a starting value. Maybe we just need to make the comparison below explicitly signed with a cast.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
while (true) {<br>
- /* First, we find which thread has the highest next element */<br>
- int thread_max = -1;<br>
+ /* First, we find which thread has the lowest next element */<br>
+ uint32_t thread_max = UINT32_MAX;<br></blockquote><div><br></div><div>Yes, this loop needs to change. However, I think we should rename the variables to thread_min and min_thread_idx.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
int max_thread_idx = -1;<br>
for (unsigned i = 0; i < NUM_THREADS; i++) {<br>
if (next[i] >= BLOCKS_PER_THREAD)<br>
continue;<br>
<br>
- if (thread_max < blocks[i][next[i]]) {<br>
+ if (thread_max > blocks[i][next[i]]) {<br>
thread_max = blocks[i][next[i]];<br>
max_thread_idx = i;<br>
}<br>
@@ -96,13 +96,14 @@ static void validate_monotonic(uint32_t **blocks)<br>
/* The only way this can happen is if all of the next[] values are at<br>
* BLOCKS_PER_THREAD, in which case, we're done.<br>
*/<br>
- if (thread_max == -1)<br>
+ if (thread_max == UINT32_MAX)<br>
break;<br>
<br>
- /* That next element had better be higher than the previous highest */<br>
- assert(blocks[max_thread_idx][next[max_thread_idx]] > highest);<br>
+ /* That next element had better be higher than the previous lowest */<br>
+ assert(lowest == UINT32_MAX ||<br>
+ blocks[max_thread_idx][next[max_thread_idx]] > lowest);<br>
<br>
- highest = blocks[max_thread_idx][next[max_thread_idx]];<br>
+ lowest = blocks[max_thread_idx][next[max_thread_idx]];<br>
next[max_thread_idx]++;<br>
}<br>
}<br>
-- <br>
2.17.1<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>