[Mesa-dev] [RFC PATCH] nir: call nir_validate_shader in debug but not debugoptimized builds

[Jason Ekstrand]

Please, no.  You could make a case for changing the default in 
debugoptimized builds (which I would still be against) but we should 
definitely still compile it. Why is it so hard to set NIR_VALIDATE=0 when 
you really care about compiler times?

--Jason


On May 10, 2019 20:15:56 Marek Olšák <maraeo at gmail.com> wrote:

> From: Marek Olšák <marek.olsak at amd.com>
>
>
> This reverts commit 7b85b9b8773b119360a31b66b321ae560a77cb6d.
> ---
> src/compiler/nir/nir.h          | 8 ++++----
> src/compiler/nir/nir_metadata.c | 2 +-
> src/compiler/nir/nir_validate.c | 2 +-
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 8441c9f26c5..6ee733efa75 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -35,23 +35,23 @@
> #include "util/ralloc.h"
> #include "util/set.h"
> #include "util/bitscan.h"
> #include "util/bitset.h"
> #include "util/macros.h"
> #include "compiler/nir_types.h"
> #include "compiler/shader_enums.h"
> #include "compiler/shader_info.h"
> #include <stdio.h>
>
> -#ifndef NDEBUG
> +#ifdef DEBUG
> #include "util/debug.h"
> -#endif /* NDEBUG */
> +#endif /* DEBUG */
>
> #include "nir_opcodes.h"
>
> #if defined(_WIN32) && !defined(snprintf)
> #define snprintf _snprintf
> #endif
>
> #ifdef __cplusplus
> extern "C" {
> #endif
> @@ -2875,21 +2875,21 @@ void nir_print_instr(const nir_instr *instr, FILE *fp);
> void nir_print_deref(const nir_deref_instr *deref, FILE *fp);
>
> nir_shader *nir_shader_clone(void *mem_ctx, const nir_shader *s);
> nir_function_impl *nir_function_impl_clone(nir_shader *shader,
>                                            const nir_function_impl *fi);
> nir_constant *nir_constant_clone(const nir_constant *c, nir_variable *var);
> nir_variable *nir_variable_clone(const nir_variable *c, nir_shader *shader);
>
> nir_shader *nir_shader_serialize_deserialize(void *mem_ctx, nir_shader *s);
>
> -#ifndef NDEBUG
> +#ifdef DEBUG
> void nir_validate_shader(nir_shader *shader, const char *when);
> void nir_metadata_set_validation_flag(nir_shader *shader);
> void nir_metadata_check_validation_flag(nir_shader *shader);
>
> static inline bool
> should_skip_nir(const char *name)
> {
>    static const char *list = NULL;
>    if (!list) {
>       /* Comma separated list of names to skip. */
> @@ -2934,21 +2934,21 @@ should_print_nir(void)
>    return should_print;
> }
> #else
> static inline void nir_validate_shader(nir_shader *shader, const char 
> *when) { (void) shader; (void)when; }
> static inline void nir_metadata_set_validation_flag(nir_shader *shader) { 
> (void) shader; }
> static inline void nir_metadata_check_validation_flag(nir_shader *shader) { 
> (void) shader; }
> static inline bool should_skip_nir(UNUSED const char *pass_name) { return 
> false; }
> static inline bool should_clone_nir(void) { return false; }
> static inline bool should_serialize_deserialize_nir(void) { return false; }
> static inline bool should_print_nir(void) { return false; }
> -#endif /* NDEBUG */
> +#endif /* DEBUG */
>
> #define _PASS(pass, nir, do_pass) do {                               \
>    if (should_skip_nir(#pass)) {                                     \
>       printf("skipping %s\n", #pass);                                \
>       break;                                                         \
>    }                                                                 \
>    do_pass                                                           \
>    nir_validate_shader(nir, "after " #pass);                         \
>    if (should_clone_nir()) {                                         \
>       nir_shader *clone = nir_shader_clone(ralloc_parent(nir), nir); \
> diff --git a/src/compiler/nir/nir_metadata.c b/src/compiler/nir/nir_metadata.c
> index e681ba34f75..f71cf432b70 100644
> --- a/src/compiler/nir/nir_metadata.c
> +++ b/src/compiler/nir/nir_metadata.c
> @@ -52,21 +52,21 @@ nir_metadata_require(nir_function_impl *impl, 
> nir_metadata required, ...)
>
>    impl->valid_metadata |= required;
> }
>
> void
> nir_metadata_preserve(nir_function_impl *impl, nir_metadata preserved)
> {
>    impl->valid_metadata &= preserved;
> }
>
> -#ifndef NDEBUG
> +#ifdef DEBUG
> /**
>  * Make sure passes properly invalidate metadata (part 1).
>  *
>  * Call this before running a pass to set a bogus metadata flag, which will
>  * only be preserved if the pass forgets to call nir_metadata_preserve().
>  */
> void
> nir_metadata_set_validation_flag(nir_shader *shader)
> {
>    nir_foreach_function(function, shader) {
> diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
> index 7746c391abc..a57b78e97c0 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -29,21 +29,21 @@
> #include "c11/threads.h"
> #include <assert.h>
>
> /*
>  * This file checks for invalid IR indicating a bug somewhere in the compiler.
>  */
>
> /* Since this file is just a pile of asserts, don't bother compiling it if
>  * we're not building a debug build.
>  */
> -#ifndef NDEBUG
> +#ifdef DEBUG
>
> /*
>  * Per-register validation state.
>  */
>
> typedef struct {
>    /*
>     * equivalent to the uses and defs in nir_register, but built up by the
>     * validator. At the end, we verify that the sets have the same entries.
>     */
> --
> 2.17.1
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev