[Mesa-dev] [PATCH 2/5] i965: Initialize compaction tables once per process.

[Jason Ekstrand]

On Wed, Nov 26, 2014 at 11:10 AM, Matt Turner <mattst88 at gmail.com> wrote:

> On Wed, Nov 26, 2014 at 10:59 AM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > On Wed, Nov 26, 2014 at 10:39 AM, Matt Turner <mattst88 at gmail.com>
> wrote:
> >>
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_eu_compact.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> >> b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> >> index 7117890..8e33bcb 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> >> @@ -75,6 +75,7 @@
> >>  #include "brw_context.h"
> >>  #include "brw_eu.h"
> >>  #include "intel_asm_annotation.h"
> >> +#include "util/u_atomic.h" /* for p_atomic_cmpxchg */
> >>
> >>  static const uint32_t g45_control_index_table[32] = {
> >>     0b00000000000000000,
> >> @@ -1247,6 +1248,10 @@ update_gen4_jump_count(struct brw_context *brw,
> >> brw_inst *insn,
> >>  void
> >>  brw_init_compaction_tables(struct brw_context *brw)
> >>  {
> >> +   static bool initialized;
> >> +   if (initialized || p_atomic_cmpxchg(&initialized, false, true) !=
> >> false)
> >> +      return;
> >> +
> >
> >
> > Sure, this protects the initialized flag, but what happens if a thread
> tries
> > to use compaction after someone else starts initializing but before
> they've
> > finished?  Same comment for the other two patches that do more-or-less
> the
> > same thing.
>
> Yes, that is theoretically possible.
>
> But for it to happen two threads would have to arrive at
> brw_init_compaction_tables() at the same time, one would get through
> the initialized lock and the other wouldn't. The one that didn't would
> then have to compile an entire shader with everything that entails,
> run the generator, and then get to compacting the instructions before
> the initializing thread executed the 27 instructions in this function.
>

Sure, not likely.  I'd still throw in a comment to future readers as to why
its 99.9% safe.


>
> Since the initializations in the other two patches are just single
> variables (i.e., not a variable protecting a whole initialization
> function) I don't think they even have the same theoretical problem.
>

Right.  Wasn't reading close enough.  With the comment above, you can have
my R-B on the rest of them.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141126/d9577a9b/attachment.html>