On Mon, Aug 21, 2017 at 2:44 PM, Matt Turner <matts...@gmail.com> wrote:
> On Thu, Aug 10, 2017 at 7:02 PM, Jordan Justen
> <jordan.l.jus...@intel.com> wrote:
> > On 2017-08-10 15:02:33, Matt Turner wrote:
> >> Quiets a number of uninitialized variable warnings in clang.
> >> ---
> >> src/compiler/spirv/spirv_to_nir.c | 24 ++++++++++++------------
> >> src/compiler/spirv/vtn_variables.c | 10 +++++-----
> >> 2 files changed, 17 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> >> index 7b34dad30c..870dda0314 100644
> >> --- a/src/compiler/spirv/spirv_to_nir.c
> >> +++ b/src/compiler/spirv/spirv_to_nir.c
> >> @@ -262,7 +262,7 @@ vtn_handle_extension(struct vtn_builder *b, SpvOp
> opcode,
> >> if (strcmp((const char *)&w[2], "GLSL.std.450") == 0) {
> >> val->ext_handler = vtn_handle_glsl450_instruction;
> >> } else {
> >> - assert(!"Unsupported extension");
> >> + unreachable("Unsupported extension");
> >> }
> >> break;
> >> }
> >> @@ -724,7 +724,7 @@ translate_image_format(SpvImageFormat format)
> >> case SpvImageFormatR16ui: return 0x8234; /* GL_R16UI */
> >> case SpvImageFormatR8ui: return 0x8232; /* GL_R8UI */
> >> default:
> >> - assert(!"Invalid image format");gl_primitive_from_
> spv_execution_mode
> >> + unreachable("Invalid image format");
> >> return 0;
> >> }
> >> }
> >> @@ -785,7 +785,7 @@ vtn_handle_type(struct vtn_builder *b, SpvOp opcode,
> >> val->type->type = glsl_matrix_type(glsl_get_
> base_type(base->type),
> >> glsl_get_vector_elements(base-
> >type),
> >> columns);
> >> - assert(!glsl_type_is_error(val->type->type));
> >> + unreachable(glsl_type_is_error(val->type->type));
> >
> > I think we want assert here, right?
>
> Yes. How sloppy. Thanks.
>
> I was hoping I could get a comment from Jason. There seem to be vague
> attempts to keep things going even after an assert (return 4 in
> gl_primitive_from_spv_execution_mode for instance). Are you okay with
> making these unreachables? If so, I'll respin the patch and drop that
> now-dead code as well as fixing the still-should-be-asserts noted by
> Jordan.
>
I'm fine with it. I have some patches on the list to add a new vtn_fail
function which longjumps and then cleans up in which case, we'll replace
the unreachable() with vtn_fail(). However, it's marked __noreturn__ so
clang should also know that it's not a fall-through in that case as well.
--Jason
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
