On 01/01/2018 08:09 PM, Jason Ekstrand wrote:
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104424
> ---
>  src/compiler/spirv/vtn_variables.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/src/compiler/spirv/vtn_variables.c 
> b/src/compiler/spirv/vtn_variables.c
> index d69b056..48797f6 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -1899,6 +1899,28 @@ vtn_create_variable(struct vtn_builder *b, struct 
> vtn_value *val,
>     }
>  }
>  
> +static void
> +vtn_assert_types_equal(struct vtn_builder *b, SpvOp opcode,
> +                       struct vtn_type *dst_type, struct vtn_type *src_type)
> +{
> +   if (dst_type->val == src_type->val)
> +      return;
> +
> +   if (dst_type->type == src_type->type) {
> +      /* Early versions of GLSLang would re-emit types unnecessarily and you
> +       * would end up with OpLoad, OpStore, or OpCopyMemory opcodes which 
> have
> +       * mismatched source and destination types.
> +       */
> +      vtn_warn("Source and destination types of %s do not match",
> +               spirv_op_to_string(opcode));

This is deep compare vs. shallow compare, right?  Looking at the SPIR-V
spec, it's not clear to me which kind of "equality" is necessary.  What
does the validator do?  I'm just wondering of we should even bother
emitting a warning since this may just be a sub-optimal SPIR-V binary.
Emitting this particular warning here is a bit misleading.  The real
problem is that there are multiple identical types with different names,
right?

> +   } else {
> +      vtn_fail("Source and destination types of %s do not match: %s vs. %s",
> +               spirv_op_to_string(opcode),
> +               glsl_get_type_name(dst_type->type),
> +               glsl_get_type_name(src_type->type));
> +   }
> +}
> +
>  void
>  vtn_handle_variables(struct vtn_builder *b, SpvOp opcode,
>                       const uint32_t *w, unsigned count)
> @@ -1975,8 +1997,7 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
>        struct vtn_value *dest = vtn_value(b, w[1], vtn_value_type_pointer);
>        struct vtn_value *src = vtn_value(b, w[2], vtn_value_type_pointer);
>  
> -      vtn_fail_if(dest->type->deref != src->type->deref,
> -                  "Dereferenced pointer types to OpCopyMemory do not match");
> +      vtn_assert_types_equal(b, opcode, dest->type->deref, src->type->deref);
>  
>        vtn_variable_copy(b, dest->pointer, src->pointer);
>        break;
> @@ -1988,8 +2009,7 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
>        struct vtn_value *src_val = vtn_value(b, w[3], vtn_value_type_pointer);
>        struct vtn_pointer *src = src_val->pointer;
>  
> -      vtn_fail_if(res_type != src_val->type->deref,
> -                  "Result and pointer types of OpLoad do not match");
> +      vtn_assert_types_equal(b, opcode, res_type, src_val->type->deref);
>  
>        if (src->mode == vtn_variable_mode_image ||
>            src->mode == vtn_variable_mode_sampler) {
> @@ -2006,8 +2026,7 @@ vtn_handle_variables(struct vtn_builder *b, SpvOp 
> opcode,
>        struct vtn_pointer *dest = dest_val->pointer;
>        struct vtn_value *src_val = vtn_untyped_value(b, w[2]);
>  
> -      vtn_fail_if(dest_val->type->deref != src_val->type,
> -                  "Value and pointer types of OpStore do not match");
> +      vtn_assert_types_equal(b, opcode, dest_val->type->deref, 
> src_val->type);
>  
>        if (glsl_type_is_sampler(dest->type->type)) {
>           vtn_warn("OpStore of a sampler detected.  Doing on-the-fly copy "
> 

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to