From 7adf6c75efa215d59d466800ddd7a90065157d8b Mon Sep 17 00:00:00 2001 From: Caio Oliveira Date: Thu, 11 May 2023 17:05:26 -0700 Subject: [PATCH] spirv: Improve the 'ID is the wrong kind of value' error messages Include the expected and actual values in the errors -- since very frequently we care about them to diagnose issues. Since these helpers are meant to be inlined, also pull the failure code out of the way into a separate function (not meant to be inlined). This way, extra calls to to_string will not harm the existing client code size. Verified this with GCC release build. Reviewed-by: Faith Ekstrand Part-of: --- src/compiler/spirv/spirv_to_nir.c | 47 +++++++++++++++++++++++++++++++++++++++ src/compiler/spirv/vtn_private.h | 21 ++++++++++++----- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c index 11c12ce..41e3719 100644 --- a/src/compiler/spirv/spirv_to_nir.c +++ b/src/compiler/spirv/spirv_to_nir.c @@ -212,6 +212,53 @@ _vtn_fail(struct vtn_builder *b, const char *file, unsigned line, vtn_longjmp(b->fail_jump, 1); } +const char * +vtn_value_type_to_string(enum vtn_value_type t) +{ +#define CASE(typ) case vtn_value_type_##typ: return #typ + switch (t) { + CASE(invalid); + CASE(undef); + CASE(string); + CASE(decoration_group); + CASE(type); + CASE(constant); + CASE(pointer); + CASE(function); + CASE(block); + CASE(ssa); + CASE(extension); + CASE(image_pointer); + } +#undef CASE + unreachable("unknown value type"); + return "UNKNOWN"; +} + +void +_vtn_fail_value_type_mismatch(struct vtn_builder *b, uint32_t value_id, + enum vtn_value_type value_type) +{ + struct vtn_value *val = vtn_untyped_value(b, value_id); + vtn_fail( + "SPIR-V id %u is the wrong kind of value: " + "expected '%s' but got '%s'", + vtn_id_for_value(b, val), + vtn_value_type_to_string(value_type), + vtn_value_type_to_string(val->value_type)); +} + +void _vtn_fail_value_not_pointer(struct vtn_builder *b, + uint32_t value_id) +{ + struct vtn_value *val = vtn_untyped_value(b, value_id); + vtn_fail("SPIR-V id %u is the wrong kind of value: " + "expected 'pointer' OR null constant but got " + "'%s' (%s)", value_id, + vtn_value_type_to_string(val->value_type), + val->is_null_constant ? "null constant" : "not null constant"); +} + static struct vtn_ssa_value * vtn_undef_ssa_value(struct vtn_builder *b, const struct glsl_type *type) { diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_private.h index 0f55936..e3b7b67 100644 --- a/src/compiler/spirv/vtn_private.h +++ b/src/compiler/spirv/vtn_private.h @@ -136,6 +136,8 @@ enum vtn_value_type { vtn_value_type_image_pointer, }; +const char *vtn_value_type_to_string(enum vtn_value_type t); + struct vtn_case { struct list_head link; @@ -746,13 +748,22 @@ vtn_push_value(struct vtn_builder *b, uint32_t value_id, return &b->values[value_id]; } +/* These separated fail functions exist so the helpers like vtn_value() + * can be inlined with minimal code size impact. This allows the failure + * handling to have more detailed output without harming callers. + */ + +void _vtn_fail_value_type_mismatch(struct vtn_builder *b, uint32_t value_id, + enum vtn_value_type value_type); +void _vtn_fail_value_not_pointer(struct vtn_builder *b, uint32_t value_id); + static inline struct vtn_value * vtn_value(struct vtn_builder *b, uint32_t value_id, enum vtn_value_type value_type) { struct vtn_value *val = vtn_untyped_value(b, value_id); - vtn_fail_if(val->value_type != value_type, - "SPIR-V id %u is the wrong kind of value", value_id); + if (unlikely(val->value_type != value_type)) + _vtn_fail_value_type_mismatch(b, value_id, value_type); return val; } @@ -760,9 +771,9 @@ static inline struct vtn_value * vtn_pointer_value(struct vtn_builder *b, uint32_t value_id) { struct vtn_value *val = vtn_untyped_value(b, value_id); - vtn_fail_if(val->value_type != vtn_value_type_pointer && - !val->is_null_constant, - "SPIR-V id %u is the wrong kind of value", value_id); + if (unlikely(val->value_type != vtn_value_type_pointer && + !val->is_null_constant)) + _vtn_fail_value_not_pointer(b, value_id); return val; } -- 2.7.4