On 30/11/17 21:24, Jason Ekstrand wrote: > I sprinkled a few mostly trivial comments below. With those fixed, > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net > <mailto:ja...@jlekstrand.net>> > > On Wed, Nov 29, 2017 at 6:07 PM, Jose Maria Casanova Crespo > <jmcasan...@igalia.com <mailto:jmcasan...@igalia.com>> wrote: > > From: Eduardo Lima Mitev <el...@igalia.com <mailto:el...@igalia.com>> > > v2: Added more missing implementations of 16-bit types. (Jason Ekstrand) > > v3: Store values in values[0].u16[i] (Jason Ekstrand) > Include switches based on bitsize for 16-bit types > (Chema Casanova) > > Signed-off-by: Jose Maria Casanova Crespo <jmcasan...@igalia.com > <mailto:jmcasan...@igalia.com>> > Signed-off-by: Eduardo Lima <el...@igalia.com <mailto:el...@igalia.com>> > --- > src/compiler/spirv/spirv_to_nir.c | 111 > +++++++++++++++++++++++++++++++------ > src/compiler/spirv/vtn_variables.c | 21 +++++++ > 2 files changed, 115 insertions(+), 17 deletions(-) > > diff --git a/src/compiler/spirv/spirv_to_nir.c > b/src/compiler/spirv/spirv_to_nir.c > index 027efab88d..f745373473 100644 > --- a/src/compiler/spirv/spirv_to_nir.c > +++ b/src/compiler/spirv/spirv_to_nir.c > @@ -104,10 +104,13 @@ vtn_const_ssa_value(struct vtn_builder *b, > nir_constant *constant, > switch (glsl_get_base_type(type)) { > case GLSL_TYPE_INT: > case GLSL_TYPE_UINT: > + case GLSL_TYPE_INT16: > + case GLSL_TYPE_UINT16: > case GLSL_TYPE_INT64: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: { > int bit_size = glsl_get_bit_size(type); > if (glsl_type_is_vector_or_scalar(type)) { > @@ -751,16 +754,38 @@ vtn_handle_type(struct vtn_builder *b, SpvOp > opcode, > int bit_size = w[2]; > const bool signedness = w[3]; > val->type->base_type = vtn_base_type_scalar; > - if (bit_size == 64) > + switch (bit_size) { > + case 64: > val->type->type = (signedness ? glsl_int64_t_type() : > glsl_uint64_t_type()); > - else > + break; > + case 32: > val->type->type = (signedness ? glsl_int_type() : > glsl_uint_type()); > + break; > + case 16: > + val->type->type = (signedness ? glsl_int16_t_type() : > glsl_uint16_t_type()); > + break; > + default: > + unreachable("Invalid int bit size"); > + } > break; > } > + > case SpvOpTypeFloat: { > int bit_size = w[2]; > val->type->base_type = vtn_base_type_scalar; > - val->type->type = bit_size == 64 ? glsl_double_type() : > glsl_float_type(); > + switch (bit_size) { > + case 16: > + val->type->type = glsl_float16_t_type(); > + break; > + case 32: > + val->type->type = glsl_float_type(); > + break; > + case 64: > + val->type->type = glsl_double_type(); > + break; > + default: > + assert(!"Invalid float bit size"); > > > unreachable()
Fixed locally. > + } > break; > } > > @@ -980,10 +1005,13 @@ vtn_null_constant(struct vtn_builder *b, > const struct glsl_type *type) > switch (glsl_get_base_type(type)) { > case GLSL_TYPE_INT: > case GLSL_TYPE_UINT: > + case GLSL_TYPE_INT16: > + case GLSL_TYPE_UINT16: > case GLSL_TYPE_INT64: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > /* Nothing to do here. It's already initialized to zero */ > break; > @@ -1106,12 +1134,20 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > case SpvOpConstant: { > assert(glsl_type_is_scalar(val->const_type)); > int bit_size = glsl_get_bit_size(val->const_type); > - if (bit_size == 64) { > + switch (bit_size) { > + case 64: { > val->constant->values->u32[0] = w[3]; > val->constant->values->u32[1] = w[4]; > > > A bit unrelated but this should be using vtn_u64_literal and setting > u64[0] instead of assuming the aliasing works out between u32 and u64. Let it be: val->constant->values->u64[0] = vtn_u64_literal(&w[3]); > - } else { > - assert(bit_size == 32); > + break; > + } > > > You don't need braces around the 64-bit case. Ok. > + case 32: > val->constant->values->u32[0] = w[3]; > + break; > + case 16: > + val->constant->values->u16[0] = w[3]; > + break; > + default: > + unreachable("Unsupported SpvOpConstant bit size"); > } > break; > } > @@ -1119,11 +1155,21 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > assert(glsl_type_is_scalar(val->const_type)); > val->constant->values[0].u32[0] = get_specialization(b, val, > w[3]); > int bit_size = glsl_get_bit_size(val->const_type); > - if (bit_size == 64) > + switch (bit_size) { > + case 64:{ > val->constant->values[0].u64[0] = > get_specialization64(b, val, vtn_u64_literal(&w[3])); > - else > + break; > + } > > > Same here. Ok. Thanks for the review. > > > + case 32: > val->constant->values[0].u32[0] = get_specialization(b, > val, w[3]); > + break; > + case 16: > + val->constant->values[0].u16[0] = get_specialization(b, > val, w[3]); > + break; > + default: > + unreachable("Unsupported SpvOpSpecConstant bit size"); > + } > break; > } > case SpvOpSpecConstantComposite: > @@ -1136,9 +1182,12 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > switch (glsl_get_base_type(val->const_type)) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_DOUBLE: { > int bit_size = glsl_get_bit_size(val->const_type); > @@ -1150,11 +1199,18 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > assert(glsl_type_is_vector(val->const_type)); > assert(glsl_get_vector_elements(val->const_type) == > elem_count); > for (unsigned i = 0; i < elem_count; i++) { > - if (bit_size == 64) { > + switch (bit_size) { > + case 64: > val->constant->values[0].u64[i] = > elems[i]->values[0].u64[0]; > - } else { > - assert(bit_size == 32); > + break; > + case 32: > val->constant->values[0].u32[i] = > elems[i]->values[0].u32[0]; > + break; > + case 16: > + val->constant->values[0].u16[i] = > elems[i]->values[0].u16[0]; > + break; > + default: > + unreachable("Invalid SpvOpConstantComposite bit > size"); > } > } > } > @@ -1228,6 +1284,7 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > val->constant->values[0].u64[j] = u64[comp]; > } > } else { > + /* This is for both 32-bit and 16-bit values */ > uint32_t u32[8]; > if (v0->value_type == vtn_value_type_constant) { > for (unsigned i = 0; i < len0; i++) > @@ -1276,9 +1333,12 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > switch (glsl_get_base_type(type)) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > /* If we hit this granularity, we're picking off an > element */ > @@ -1316,11 +1376,18 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > unsigned num_components = > glsl_get_vector_elements(type); > unsigned bit_size = glsl_get_bit_size(type); > for (unsigned i = 0; i < num_components; i++) > - if (bit_size == 64) { > + switch(bit_size) { > + case 64: > val->constant->values[0].u64[i] = > (*c)->values[col].u64[elem + i]; > - } else { > - assert(bit_size == 32); > + break; > + case 32: > val->constant->values[0].u32[i] = > (*c)->values[col].u32[elem + i]; > + break; > + case 16: > + val->constant->values[0].u16[i] = > (*c)->values[col].u16[elem + i]; > + break; > + default: > + unreachable("Invalid SpvOpCompositeExtract bit > size"); > } > } > } else { > @@ -1333,11 +1400,18 @@ vtn_handle_constant(struct vtn_builder *b, > SpvOp opcode, > unsigned num_components = > glsl_get_vector_elements(type); > unsigned bit_size = glsl_get_bit_size(type); > for (unsigned i = 0; i < num_components; i++) > - if (bit_size == 64) { > + switch (bit_size) { > + case 64: > (*c)->values[col].u64[elem + i] = > insert->constant->values[0].u64[i]; > - } else { > - assert(bit_size == 32); > + break; > + case 32: > (*c)->values[col].u32[elem + i] = > insert->constant->values[0].u32[i]; > + break; > + case 16: > + (*c)->values[col].u16[elem + i] = > insert->constant->values[0].u16[i]; > + break; > + default: > + unreachable("Invalid SpvOpCompositeInsert bit > size"); > } > } > } > @@ -1449,10 +1523,13 @@ vtn_create_ssa_value(struct vtn_builder *b, > const struct glsl_type *type) > switch (glsl_get_base_type(type)) { > case GLSL_TYPE_INT: > case GLSL_TYPE_UINT: > + case GLSL_TYPE_INT16: > + case GLSL_TYPE_UINT16: > case GLSL_TYPE_INT64: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > child_type = glsl_get_column_type(type); > break; > diff --git a/src/compiler/spirv/vtn_variables.c > b/src/compiler/spirv/vtn_variables.c > index 9a69b4f6fc..7eb0b6d22a 100644 > --- a/src/compiler/spirv/vtn_variables.c > +++ b/src/compiler/spirv/vtn_variables.c > @@ -186,9 +186,12 @@ vtn_ssa_offset_pointer_dereference(struct > vtn_builder *b, > switch (glsl_get_base_type(type->type)) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_ARRAY: { > @@ -298,9 +301,12 @@ vtn_pointer_to_deref(struct vtn_builder *b, > struct vtn_pointer *ptr) > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_ARRAY: { > @@ -523,9 +529,12 @@ vtn_pointer_to_offset(struct vtn_builder *b, > struct vtn_pointer *ptr, > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_ARRAY: > @@ -567,9 +576,12 @@ vtn_type_block_size(struct vtn_type *type) > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_DOUBLE: { > unsigned cols = type->row_major ? > glsl_get_vector_elements(type->type) : > @@ -698,9 +710,12 @@ _vtn_block_load_store(struct vtn_builder *b, > nir_intrinsic_op op, bool load, > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > /* This is where things get interesting. At this point, > we've hit > @@ -873,9 +888,12 @@ _vtn_variable_load_store(struct vtn_builder *b, > bool load, > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_BOOL: > case GLSL_TYPE_DOUBLE: > /* At this point, we have a scalar, vector, or matrix so we > know that > @@ -953,9 +971,12 @@ _vtn_variable_copy(struct vtn_builder *b, > struct vtn_pointer *dest, > switch (base_type) { > case GLSL_TYPE_UINT: > case GLSL_TYPE_INT: > + case GLSL_TYPE_UINT16: > + case GLSL_TYPE_INT16: > case GLSL_TYPE_UINT64: > case GLSL_TYPE_INT64: > case GLSL_TYPE_FLOAT: > + case GLSL_TYPE_FLOAT16: > case GLSL_TYPE_DOUBLE: > case GLSL_TYPE_BOOL: > /* At this point, we have a scalar, vector, or matrix so we > know that > -- > 2.14.3 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > <https://lists.freedesktop.org/mailman/listinfo/mesa-dev> > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev