<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Sun, Jul 15, 2018 at 3:11 AM Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl">bas@basnieuwenhuizen.nl</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Sun, Jul 15, 2018 at 3:56 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: > For one thing, the NIR opcodes for image load/store always take and > return a vec4 value regardless of the image type. We need to fix up > both the source and destination to handle it. For another thing, we > weren't actually setting up a destination in the OpAtomicLoad case. Nominate for stable? I actually encountered this in an application. > --- > src/compiler/spirv/spirv_to_nir.c | 44 +++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 17 deletions(-) > > diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c > index c31293a77be..a204bfa39ba 100644 > --- a/src/compiler/spirv/spirv_to_nir.c > +++ b/src/compiler/spirv/spirv_to_nir.c > @@ -2309,6 +2309,15 @@ get_image_coord(struct vtn_builder *b, uint32_t value) > return nir_swizzle(&b->nb, coord->def, swizzle, 4, false); > } > > +static nir_ssa_def * > +expand_to_vec4(nir_builder *b, nir_ssa_def *value) > +{ > + unsigned swiz[4]; > + for (unsigned i = 0; i < 4; i++) > + swiz[i] = i < value->num_components ? i : 0; > + return nir_swizzle(b, value, swiz, 4, false); Maybe skip if value already has 4 components? </blockquote><div> </div><div>Done. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Either way, </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Reviewed-by: Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> </blockquote><div> </div><div>Thanks! I'll push in the morning assuming my Jenkins run is happy.</div><div> </div><div>--Uason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > +} > + > static void > vtn_handle_image(struct vtn_builder *b, SpvOp opcode, > const uint32_t *w, unsigned count) > @@ -2422,11 +2431,7 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode, > /* The image coordinate is always 4 components but we may not have that > * many. Swizzle to compensate. > */ > - unsigned swiz[4]; > - for (unsigned i = 0; i < 4; i++) > - swiz[i] = i < image.coord->num_components ? i : 0; > - intrin->src[1] = nir_src_for_ssa(nir_swizzle(&b->nb, image.coord, > - swiz, 4, false)); > + intrin->src[1] = nir_src_for_ssa(expand_to_vec4(&b->nb, image.coord)); > intrin->src[2] = nir_src_for_ssa(image.sample); > } > > @@ -2436,11 +2441,13 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode, > case SpvOpImageRead: > break; > case SpvOpAtomicStore: > - intrin->src[3] = nir_src_for_ssa(vtn_ssa_value(b, w[4])->def); > - break; > - case SpvOpImageWrite: > - intrin->src[3] = nir_src_for_ssa(vtn_ssa_value(b, w[3])->def); > + case SpvOpImageWrite: { > + const uint32_t value_id = opcode == SpvOpAtomicStore ? w[4] : w[3]; > + nir_ssa_def *value = vtn_ssa_value(b, value_id)->def; > + /* nir_intrinsic_image_deref_store always takes a vec4 value */ > + intrin->src[3] = nir_src_for_ssa(expand_to_vec4(&b->nb, value)); > break; > + } > > case SpvOpAtomicCompareExchange: > case SpvOpAtomicIIncrement: > @@ -2462,23 +2469,26 @@ vtn_handle_image(struct vtn_builder *b, SpvOp opcode, > vtn_fail("Invalid image opcode"); > } > > - if (opcode != SpvOpImageWrite) { > + if (opcode != SpvOpImageWrite && opcode != SpvOpAtomicStore) { > struct vtn_value *val = vtn_push_value(b, w[2], vtn_value_type_ssa); > struct vtn_type *type = vtn_value(b, w[1], vtn_value_type_type)->type; > > - unsigned dest_components = nir_intrinsic_dest_components(intrin); > - if (intrin->intrinsic == nir_intrinsic_image_deref_size) { > - dest_components = intrin->num_components = > - glsl_get_vector_elements(type->type); > - } > + unsigned dest_components = glsl_get_vector_elements(type->type); > + intrin->num_components = nir_intrinsic_infos[op].dest_components; > + if (intrin->num_components == 0) > + intrin->num_components = dest_components; > > nir_ssa_dest_init(&intrin->instr, &intrin->dest, > - dest_components, 32, NULL); > + intrin->num_components, 32, NULL); > > nir_builder_instr_insert(&b->nb, &intrin->instr); > > + nir_ssa_def *result = &intrin->dest.ssa; > + if (intrin->num_components != dest_components) > + result = nir_channels(&b->nb, result, (1 << dest_components) - 1); > + > val->ssa = vtn_create_ssa_value(b, type->type); > - val->ssa->def = &intrin->dest.ssa; > + val->ssa->def = result; > } else { > nir_builder_instr_insert(&b->nb, &intrin->instr); > } > -- > 2.17.1 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">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>