On Wed, Nov 26, 2014 at 11:10 AM, Matt Turner <matts...@gmail.com> wrote:
> On Wed, Nov 26, 2014 at 10:59 AM, Jason Ekstrand <ja...@jlekstrand.net>
> wrote:
> > On Wed, Nov 26, 2014 at 10:39 AM, Matt Turner <matts...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
