<div dir="ltr"><div>Baseline: No compiler produces optimal code in all cases - it's too expensive. They'll all have some tradeoffs, some more reasonable than others, some more intentional than others. I don't know where this particular situation lies (whether it's a reasonable tradeoff of compiler complexity, or an accidental one that might still be a reasonable tradeoff of compiler engineer time (other things more important than this particular optimization, etc)). </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 25, 2019 at 1:02 AM אלכס לופ' <<a href="mailto:alex_lop@walla.com">alex_lop@walla.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"><div dir="rtl"><div>
<div dir="rtl">
<div dir="ltr">Could be... But the wierd thing here is if I change the array to be of size 256 and the index to be 'unsigned char', seems like there is no way to access the 'size' field throught "y->ptr[index]" (in your example) but clang still performs the re-read: <a href="https://godbolt.org/z/ahlGHY" target="_blank">https://godbolt.org/z/ahlGHY</a></div></div></div></div></blockquote><div><br>Yup - I guess LLVM doesn't track/use the upper bound of the index here, even if it uses the lower bound. (as demonstrated by the reordering enabling the optimization)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="rtl"><div dir="rtl"><div dir="ltr">It looks like clang sees an alias between "int ptr[256]" and "int size"... maybe because they are both "int"? Because changing the type of size to char/short/long eliminates the re-read: <a href="https://godbolt.org/z/4kr1oo" target="_blank">https://godbolt.org/z/4kr1oo</a></div></div></div></blockquote><div><br>That's somewhat interesting - hmm, I'd assumed we didn't use strict aliasing by default, but perhaps we do. You can try compiling with -fno-strict-aliasing to disable type based alias analysis & see that the type change doesn't fix the issue then.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="rtl"><div dir="rtl"><div dir="ltr">Interesting and confusing at the same time. Can it be related to the standard definition of (non)aliasing as this answer suggests: <a href="https://stackoverflow.com/a/58550055/5218277" target="_blank">https://stackoverflow.com/a/58550055/5218277</a> ?<br></div></div></div></blockquote><div><br>Yeah, guess so - once you change the type, type based alias analysis proves the pointer produced by the index arithmetic can't alias the non-int member. (with -fno-strict-aliasing changing the type does not remove the extra load even when the type is different)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="rtl"><div dir="rtl"><div dir="ltr"></div>
<div dir="ltr"> On the other hand, placing the "int size" (as you mentioned) before "int ptr[256]" also eliminates the re-reads: <a href="https://godbolt.org/z/kpB74m" target="_blank">https://godbolt.org/z/kpB74m</a></div>
<div dir="ltr"> </div>
<div dir="ltr"> </div>
</div>
<blockquote style="margin:0px 0px 20px;border-top:1px solid rgb(224,224,224)"><br>ב אוק׳ 25, 2019 0:44, David Blaikie כתב:
<blockquote style="margin:0px 0px 20px;border-top:1px solid rgb(224,224,224)">
<div dir="ltr">
<div dir="ltr">Here's a simpler version which I believe still captures the essence of the issue: <a href="https://godbolt.org/z/BUoLq1" rel="noopener" target="_blank">https://godbolt.org/z/BUoLq1</a><br><br>Yep, looks like GCC manages this optimization and Clang does not - the runtime bound on the array is stopping clang from going further here (perhaps that's intentional - maybe to allow the array indexed store to possibly exceed the bounds of the array and assign to 'size' that comes after it - UB, but I don't know just how far non-strict aliasing goes)<br><br>Yeah, looks like it might be something like that ^ if you change the index to be unsigned, and reorder the members so 'size' comes before the array (so indexing into the array can't alias 'size'), you do get: <a href="https://godbolt.org/z/Ax62Fv" rel="noopener" target="_blank">https://godbolt.org/z/Ax62Fv</a> which avoids reloading size after the assignment to the array.</div>
<br>
<div>
<div dir="ltr">On Thu, Oct 24, 2019 at 12:53 PM אלכס לופ' <<a href="mailto:alex_lop@walla.com" rel="noopener" target="_blank">alex_lop@walla.com</a>> wrote:</div>
<blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="rtl">
<div dir="ltr">Hi David,<br><br></div>
<div dir="ltr"> </div>
<div dir="ltr">Thanks for your reply. The fact that "queue_ptr->queue[queue_ptr->wr_idx++]" could be somewhere in the object pointed by "queue_ptr" came to my mind too.</div>
<div dir="ltr">I also added this in the stackoverflow question at the bottom.</div>
<div dir="ltr">In such case, I assumed, that if I change the struct "queue_t" to have an array of "event_t" instead of just pointer to "event_t", like this:</div>
<div dir="ltr"> </div>
<div dir="ltr">
<pre><code>typedef struct
{
event_t queue[256]; // changed from pointer to array with max size
size_t size;
uint16_t num_of_items;
uint8_t rd_idx;
uint8_t wr_idx;
} queue_t;</code><br><br>there would be no valid way (without breaking the C standard definitions) that "queue_ptr->queue[queue_ptr->wr_idx++]" ends up somewhere inside "*queue_ptr", right?<br>However the generated assembly code still contained the same re-reads...<br>Any idea why would this happen even with the new "queue_t" struct definition?<br><br>P.S.<br>I can easily reproduce it and provide any additional data which can be gathered during the compilation process, if it helps.<br><br>Thanks,<br>Alex.</pre>
</div>
<div id="gmail-m_-3159906577031919118gmail-m_6399794515280957024gmail-m_-1659663878248091923gtx-trans"> <br></div>
</div>
</blockquote>
</div>
</div>
</blockquote>
</blockquote>
</div>
</blockquote></div></div>