[Mesa-dev] Let's talk about -DDEBUG

[Tapani Pälli]

On 12/14/18 12:53 PM, Erik Faye-Lund wrote:
> On Thu, 2018-12-13 at 10:46 -0800, Eric Anholt wrote:
>> Dylan Baker <dylan at 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?

Looks nice and is easy to understand. IMO something like 
'ENABLE_ASSERTS' would be also much more easier/straightforward than 
using "-Db_ndebug=false", here I'm thinking about the bug reporters.


> ---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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>