[Mesa-dev] [PATCH 02/13] nir/spirv: Use unreachable("...") rather than assert(!"...")

[Jason Ekstrand]

On Mon, Aug 21, 2017 at 2:44 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Thu, Aug 10, 2017 at 7:02 PM, Jordan Justen
> <jordan.l.justen at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170821/2a2ce365/attachment-0001.html>