<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 26, 2014 at 11:10 AM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Wed, Nov 26, 2014 at 10:59 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> On Wed, Nov 26, 2014 at 10:39 AM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>><br>
>> ---<br>
>>  src/mesa/drivers/dri/i965/brw_eu_compact.c | 5 +++++<br>
>>  1 file changed, 5 insertions(+)<br>
>><br>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c<br>
>> b/src/mesa/drivers/dri/i965/brw_eu_compact.c<br>
>> index 7117890..8e33bcb 100644<br>
>> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c<br>
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c<br>
>> @@ -75,6 +75,7 @@<br>
>>  #include "brw_context.h"<br>
>>  #include "brw_eu.h"<br>
>>  #include "intel_asm_annotation.h"<br>
>> +#include "util/u_atomic.h" /* for p_atomic_cmpxchg */<br>
>><br>
>>  static const uint32_t g45_control_index_table[32] = {<br>
>>     0b00000000000000000,<br>
>> @@ -1247,6 +1248,10 @@ update_gen4_jump_count(struct brw_context *brw,<br>
>> brw_inst *insn,<br>
>>  void<br>
>>  brw_init_compaction_tables(struct brw_context *brw)<br>
>>  {<br>
>> +   static bool initialized;<br>
>> +   if (initialized || p_atomic_cmpxchg(&initialized, false, true) !=<br>
>> false)<br>
>> +      return;<br>
>> +<br>
><br>
><br>
> Sure, this protects the initialized flag, but what happens if a thread tries<br>
> to use compaction after someone else starts initializing but before they've<br>
> finished?  Same comment for the other two patches that do more-or-less the<br>
> same thing.<br>
<br>
</div></div>Yes, that is theoretically possible.<br>
<br>
But for it to happen two threads would have to arrive at<br>
brw_init_compaction_tables() at the same time, one would get through<br>
the initialized lock and the other wouldn't. The one that didn't would<br>
then have to compile an entire shader with everything that entails,<br>
run the generator, and then get to compacting the instructions before<br>
the initializing thread executed the 27 instructions in this function.<br></blockquote><div> </div><div>Sure, not likely.  I'd still throw in a comment to future readers as to why its 99.9% safe.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Since the initializations in the other two patches are just single<br>
variables (i.e., not a variable protecting a whole initialization<br>
function) I don't think they even have the same theoretical problem.<br>
</blockquote></div><br></div><div class="gmail_extra">Right.  Wasn't reading close enough.  With the comment above, you can have my R-B on the rest of them.<br></div></div>