<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Sep 5, 2017 at 8:33 AM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">As a quick drive-by, yeah, I noticed this too, and it's going to require fixes to radv to not break things since none of the other NIR opcodes are hooked up (this will be needed for the NIR path in radeonsi too, since GLSL-to-NIR already uses those opcodes). <div><div class="h5"></div></div></blockquote><div> </div><div>Thanks for pointing that out! Dave, Bas, would either of you mind getting those properly hooked up? As a side-note, this patch also made me consider re-working the NIR barrier intrinsics to be a bit more SPIR-V like.</div><div> </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> On Tue, Sep 5, 2017 at 11:13 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > Our previous handling of barriers always used the big hammer and didn't > correctly emit memory barriers when specified along with a control > barrier. This commit completely reworks the way we emit barriers to > make things both more precise and more correct. > --- > src/compiler/spirv/spirv_to_nir.c | 132 ++++++++++++++++++++++++++++++++------ > 1 file changed, 114 insertions(+), 18 deletions(-) > > diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c > index 8653685..6fb27cb 100644 > --- a/src/compiler/spirv/spirv_to_nir.c > +++ b/src/compiler/spirv/spirv_to_nir.c > @@ -2570,36 +2570,132 @@ vtn_handle_composite(struct vtn_builder *b, SpvOp opcode, > } > > static void > +vtn_emit_barrier(struct vtn_builder *b, nir_intrinsic_op op) > +{ > + nir_intrinsic_instr *intrin = nir_intrinsic_instr_create(b->shader, op); > + nir_builder_instr_insert(&b->nb, &intrin->instr); > +} > + > +static void > +vtn_emit_memory_barrier(struct vtn_builder *b, SpvScope scope, > + SpvMemorySemanticsMask semantics) > +{ > + static const SpvMemorySemanticsMask all_memory_semantics = > + SpvMemorySemanticsUniformMemoryMask | > + SpvMemorySemanticsWorkgroupMemoryMask | > + SpvMemorySemanticsAtomicCounterMemoryMask | > + SpvMemorySemanticsImageMemoryMask; > + > + /* If we're not actually doing a memory barrier, bail */ > + if (!(semantics & all_memory_semantics)) > + return; > + > + /* GL and Vulkan don't have these */ > + assert(scope != SpvScopeCrossDevice); > + > + if (scope == SpvScopeSubgroup) > + return; /* Nothing to do here */ > + > + if (scope == SpvScopeWorkgroup) { > + vtn_emit_barrier(b, nir_intrinsic_group_memory_barrier); > + return; > + } > + > + /* There's only two scopes thing left */ > + assert(scope == SpvScopeInvocation || scope == SpvScopeDevice); > + > + if ((semantics & all_memory_semantics) == all_memory_semantics) { > + vtn_emit_barrier(b, nir_intrinsic_memory_barrier); > + return; > + } > + > + /* Issue a bunch of more specific barriers */ > + uint32_t bits = semantics; > + while (bits) { > + SpvMemorySemanticsMask semantic = 1 << u_bit_scan(&bits); > + switch (semantic) { > + case SpvMemorySemanticsUniformMemoryMask: > + vtn_emit_barrier(b, nir_intrinsic_memory_barrier_buffer); > + break; > + case SpvMemorySemanticsWorkgroupMemoryMask: > + vtn_emit_barrier(b, nir_intrinsic_memory_barrier_shared); > + break; > + case SpvMemorySemanticsAtomicCounterMemoryMask: > + vtn_emit_barrier(b, nir_intrinsic_memory_barrier_atomic_counter); > + break; > + case SpvMemorySemanticsImageMemoryMask: > + vtn_emit_barrier(b, nir_intrinsic_memory_barrier_image); > + break; > + default: > + break;; > + } > + } > +} > + > +static void > vtn_handle_barrier(struct vtn_builder *b, SpvOp opcode, > const uint32_t *w, unsigned count) > { > - nir_intrinsic_op intrinsic_op; > switch (opcode) { > case SpvOpEmitVertex: > case SpvOpEmitStreamVertex: > - intrinsic_op = nir_intrinsic_emit_vertex; > - break; > case SpvOpEndPrimitive: > - case SpvOpEndStreamPrimitive: > - intrinsic_op = nir_intrinsic_end_primitive; > - break; > - case SpvOpMemoryBarrier: > - intrinsic_op = nir_intrinsic_memory_barrier; > - break; > - case SpvOpControlBarrier: > - intrinsic_op = nir_intrinsic_barrier; > + case SpvOpEndStreamPrimitive: { > + nir_intrinsic_op intrinsic_op; > + switch (opcode) { > + case SpvOpEmitVertex: > + case SpvOpEmitStreamVertex: > + intrinsic_op = nir_intrinsic_emit_vertex; > + break; > + case SpvOpEndPrimitive: > + case SpvOpEndStreamPrimitive: > + intrinsic_op = nir_intrinsic_end_primitive; > + break; > + default: > + unreachable("Invalid opcode"); > + } > + > + nir_intrinsic_instr *intrin = > + nir_intrinsic_instr_create(b->shader, intrinsic_op); > + > + switch (opcode) { > + case SpvOpEmitStreamVertex: > + case SpvOpEndStreamPrimitive: > + nir_intrinsic_set_stream_id(intrin, w[1]); > + break; > + default: > + break; > + } > + > + nir_builder_instr_insert(&b->nb, &intrin->instr); > break; > - default: > - unreachable("unknown barrier instruction"); > } > > - nir_intrinsic_instr *intrin = > - nir_intrinsic_instr_create(b->shader, intrinsic_op); > + case SpvOpMemoryBarrier: { > + SpvScope scope = vtn_constant_value(b, w[1])->values[0].u32[0]; > + SpvMemorySemanticsMask semantics = > + vtn_constant_value(b, w[2])->values[0].u32[0]; > + vtn_emit_memory_barrier(b, scope, semantics); > + return; > + } > + > + case SpvOpControlBarrier: { > + SpvScope execution_scope = > + vtn_constant_value(b, w[1])->values[0].u32[0]; > + if (execution_scope == SpvScopeWorkgroup) > + vtn_emit_barrier(b, nir_intrinsic_barrier); > > - if (opcode == SpvOpEmitStreamVertex || opcode == SpvOpEndStreamPrimitive) > - nir_intrinsic_set_stream_id(intrin, w[1]); > + SpvScope memory_scope = > + vtn_constant_value(b, w[2])->values[0].u32[0]; > + SpvMemorySemanticsMask memory_semantics = > + vtn_constant_value(b, w[3])->values[0].u32[0]; > + vtn_emit_memory_barrier(b, memory_scope, memory_semantics); > + break; > + } > > - nir_builder_instr_insert(&b->nb, &intrin->instr); > + default: > + unreachable("unknown barrier instruction"); > + } > } > > static unsigned > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>