On Thu, 2018-12-13 at 10:46 -0800, Eric Anholt wrote: > Dylan Baker <dy...@pnwbakers.com> writes: > > > [ Unknown signature status ] > > In the autotools discussion I've come to realize that we also need > > to talk about > > the -DDEBUG guard. It seems that there are two different uses, and > > thus two > > different asks about it: > > > > - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging > > - NIR and Intel (at least) use -DDEBUG to hide really expensive > > checks that are > > useful, but necessarily tank performance. > > > > The first group would like -DDEBUG in debugoptimized builds, the > > second > > obviously doesn't. > > > > Is the right solution to move the first group being !NDEBUG, or > > would it be > > better to split DEBUG into two different defines such as > > DEBUG_MESSAGES and > > EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), > > with the > > first for both debug and debugoptimized and the second only in > > debug builds? > > I would like to see NIR validation in debugoptimized builds (which is > the build I use on a regular basis: "please catch all bugs you can at > runtime with asserts, but don't waste CPU time by compiling with > -O0"); >
I'm starting to think that we should add explicit options (with reasonable defaults based on ) for things like nir validation. That way it'd be easy for anyone to pimp their buildtype. Meddling directly with CFLAGS feels kinda hacky for something as useful like this. Something like this? ---8<--- diff --git a/meson.build b/meson.build index aba033387d3..da994e6ea87 100644 --- a/meson.build +++ b/meson.build @@ -734,6 +734,16 @@ if get_option('buildtype') == 'debug' pre_args += '-DDEBUG' endif +# define NIR_VALIDATION if needed +_nir_validation = get_option('nir-validation') +if _nir_validation == 'auto' + if get_option('buildtype') == 'debug' + pre_args += '-DNIR_VALIDATION' + endif +elif _nir_validation == 'true' + pre_args += '-DNIR_VALIDATION' +endif + if get_option('shader-cache') pre_args += '-DENABLE_SHADER_CACHE' elif with_amd_vk diff --git a/meson_options.txt b/meson_options.txt index c636b0a1099..9df20a6e080 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -318,3 +318,10 @@ option( choices : ['auto', 'true', 'false'], description : 'Enable VK_EXT_acquire_xlib_display.' ) +option( + 'nir-validation', + type : 'combo', + value : 'auto', + choices : ['auto', 'true', 'false'], + description : 'Enable automatic validation of NIR. This has a non- trivial performance penalty.' +) diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index e8be2e101cc..d5609565bea 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -2727,7 +2727,7 @@ 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 +#ifndef NIR_VALIDATION 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); @@ -2768,7 +2768,7 @@ static inline void nir_metadata_check_validation_flag(nir_shader *shader) { (voi 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 /* NIR_VALIDATION */ #define _PASS(pass, nir, do_pass) do { \ do_pass \ ---8<--- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev