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

2018-12-14 Thread Dylan Baker
Quoting Tapani Pälli (2018-12-14 05:03:06)
> 
> 
> 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  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.

I really wish meson wouldn't have called this -Db_ndebug and would have had
something like -Db_asserts, but historical artifacts live on...

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-14 Thread 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  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

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


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

2018-12-13 Thread Eric Anholt
Dylan Baker  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");


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Jason Ekstrand
On Thu, Dec 13, 2018 at 11:34 AM Dylan Baker  wrote:

> Quoting Rob Clark (2018-12-12 16:35:24)
> > On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
> > >
> > > Quoting Rob Clark (2018-12-12 15:52:47)
> > > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker 
> wrote:
> > > > >
> > > > > 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 guess my use cases for !=release builds are:
> > > >
> > > > + I want all the expensive checking because I'm not in it to win the
> > > >   deqp/piglit fps race
> > > > + I want debug syms for profiling and/or valgrind, but otherwise
> > > >   want something close to a release build but with debug syms
> > > >
> > > >
> > > > That said, I can get behind replacing DEBUG with !NDEBUG or
> > > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> > > >
> > > >
> > > > BR,
> > > > -R
> > >
> > > I guess I should have covered that:
> > >
> > > autotools had effectively two build types "debug" and "not debug",
> "debug" set
> > > "-DDEBUG -g -O2", "not debug" set -DNDEBUG
> > >
> > > Meson has 4 build types, and a separate toggle for NDEBUG:
> > > debug: -O0 -DDEBUG (we add -DDEBUG)
> > > debugoptimzed: -O2 -g
> > > release: -O2
> > > plain: (nothing)
> >
> > I generally view meson as an upgrade in this respect, I guess mostly
> > because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
> > by me).. maybe making meson debug config use -O2 would bridge the gap?
> >  If so I have no problem with dropping -O0 and making debug config
> > also use -O2
> >
> > BR.
> > -R
>
> Unfortunately buildtypes are part of meson itself, you can't override
> them. It
> does this to ensure that each buildtype is equivalent when changing
> compiler,
> ie, MSVC doesn't have -O0 and -O2, but it does have equivalent commands
> options.
>
> Basically our choices (as I see them are):
>
> 1. Status Quo
> 2. Split DEBUG into two flags, one that is only in debug builds, one that
> is in
>both debug and debugoptimized builds
>

Whatever we do, let's please not split DEBUG and NDEBUG.  We've had no end
of trouble with sorting out the fallout of them being different.


> 3. Stop defining DEBUG by default in any build type and tell people to add
>-Dc_args=-DDEBUG -Dcpp_args=-DDEBUG if they want DEBUG.
>

Can we not please?  Yes, I have a script that wraps things but
-Dbuild-type=debug should do something reasonable and reasonable includes
-DDEBUG.

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Dylan Baker
Quoting Rob Clark (2018-12-12 16:35:24)
> On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
> >
> > Quoting Rob Clark (2018-12-12 15:52:47)
> > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker  wrote:
> > > >
> > > > 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 guess my use cases for !=release builds are:
> > >
> > > + I want all the expensive checking because I'm not in it to win the
> > >   deqp/piglit fps race
> > > + I want debug syms for profiling and/or valgrind, but otherwise
> > >   want something close to a release build but with debug syms
> > >
> > >
> > > That said, I can get behind replacing DEBUG with !NDEBUG or
> > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> > >
> > >
> > > BR,
> > > -R
> >
> > I guess I should have covered that:
> >
> > autotools had effectively two build types "debug" and "not debug", "debug" 
> > set
> > "-DDEBUG -g -O2", "not debug" set -DNDEBUG
> >
> > Meson has 4 build types, and a separate toggle for NDEBUG:
> > debug: -O0 -DDEBUG (we add -DDEBUG)
> > debugoptimzed: -O2 -g
> > release: -O2
> > plain: (nothing)
> 
> I generally view meson as an upgrade in this respect, I guess mostly
> because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
> by me).. maybe making meson debug config use -O2 would bridge the gap?
>  If so I have no problem with dropping -O0 and making debug config
> also use -O2
> 
> BR.
> -R

Unfortunately buildtypes are part of meson itself, you can't override them. It
does this to ensure that each buildtype is equivalent when changing compiler,
ie, MSVC doesn't have -O0 and -O2, but it does have equivalent commands options.

Basically our choices (as I see them are):

1. Status Quo
2. Split DEBUG into two flags, one that is only in debug builds, one that is in
   both debug and debugoptimized builds
3. Stop defining DEBUG by default in any build type and tell people to add
   -Dc_args=-DDEBUG -Dcpp_args=-DDEBUG if they want DEBUG.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Axel Davy

On 13/12/2018 17:26, Jason Ekstrand wrote:
On Thu, Dec 13, 2018 at 5:06 AM Eric Engestrom 
mailto:eric.engest...@intel.com>> wrote:


On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote:
> 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?

Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I
think
the right thing would be to split it into !NDEBUG and
EXPENSIVE_VALIDATION
(the color suits me just fine :P), as both solutions satisfy the first
group but only the latter solution satisfies the 2nd group.

I think a first pass might be to simply
s/DEBUG/EXPENSIVE_VALIDATION/ so
that it expresses the intent more clearly, with a prior patch to
convert
Nine and other obvious !NDEBUG candidates, then, later on, some of the
EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case
basis.


I think this whole discussion is way over-thinking this. With autools, 
we had two options: --enable-debug or not which, as I understand it, 
corresponds to release and debug.  Great.  Now meson adds a new one.  
Let's just pick something that makes sense and call it a day; it 
really doesn't matter.  Anyone who wants more control can just set 
their own CFLAGS.  Regardless of what we do, we're not really loosing 
anything by doing this as people who build Nine today with 
--enable-debug are getting an unoptimized build the same as they would 
with -Dbuild-type=debug. Users/devs can also always set -Db_ndebug 
manually to get the behavior that they want.


I don't know that I have all that strong of a preference as I'm not 
likely to use it anyway.  On the one hand, the name implies that it's 
a debug build only optimized.  This is different than CMake's 
RelWithDebugInfo which is clearly a release build with debug symbols.  
Based on that naming, i'd say we should leave asserts on.


I think the root of the issue is that different developers have tied 
way too much stuff to -DDEBUG.  The Nine people can add a 
-Dnine-logging=true flag that can turn on logging even in release 
builds.  In the NIR-based drivers, we already have environment 
variables to shut off NIR validation to make things go faster even in 
debug builds.


--Jason


Hi,


I agree with Jason that there seems to be a multitude of needs and that 
it may be hard to handle for all these needs in a simple way.


Devs who want to stress specific parts of their code can indeed use 
CFLAGS, and thus there isn't need to have a meson build mode for every 
specific need.


However I believe using a debug build option should be all that is 
needed for a user to help report bugs. If the user is investigating a 
crash, he wants to enable asserts and debug info. He may want to enable 
nine logging, etc.
Dev flags may change between releases, while the user would always have 
the same debug option to enable all it may need.


I think the autotools way was simple for the user, and the new meson way 
should be as simple. 'debugoptimized' is counter-intuitive for an user, 
who may expect all the mentioned debug info.


To me debugoptimized should be similar to debug, but with -O2.

Other dev specific debug options can be added with CFLAGS.



Axel

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Jason Ekstrand
On Thu, Dec 13, 2018 at 5:06 AM Eric Engestrom 
wrote:

> On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote:
> > 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?
>
> Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think
> the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION
> (the color suits me just fine :P), as both solutions satisfy the first
> group but only the latter solution satisfies the 2nd group.
>
> I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so
> that it expresses the intent more clearly, with a prior patch to convert
> Nine and other obvious !NDEBUG candidates, then, later on, some of the
> EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis.
>

I think this whole discussion is way over-thinking this.  With autools, we
had two options: --enable-debug or not which, as I understand it,
corresponds to release and debug.  Great.  Now meson adds a new one.  Let's
just pick something that makes sense and call it a day; it really doesn't
matter.  Anyone who wants more control can just set their own CFLAGS.
Regardless of what we do, we're not really loosing anything by doing this
as people who build Nine today with --enable-debug are getting an
unoptimized build the same as they would with -Dbuild-type=debug.
Users/devs can also always set -Db_ndebug manually to get the behavior that
they want.

I don't know that I have all that strong of a preference as I'm not likely
to use it anyway.  On the one hand, the name implies that it's a debug
build only optimized.  This is different than CMake's RelWithDebugInfo
which is clearly a release build with debug symbols.  Based on that naming,
i'd say we should leave asserts on.

I think the root of the issue is that different developers have tied way
too much stuff to -DDEBUG.  The Nine people can add a -Dnine-logging=true
flag that can turn on logging even in release builds.  In the NIR-based
drivers, we already have environment variables to shut off NIR validation
to make things go faster even in debug builds.

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Eero Tamminen

Hi,

On 13.12.2018 12.19, Eric Engestrom wrote:

On Wednesday, 2018-12-12 19:45:06 -0500, Marek Olšák wrote:

On Wed, Dec 12, 2018 at 7:35 PM Rob Clark  wrote:

On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:

Quoting Rob Clark (2018-12-12 15:52:47)

...

I guess I should have covered that:

autotools had effectively two build types "debug" and "not debug",

"debug" set

"-DDEBUG -g -O2", "not debug" set -DNDEBUG

Meson has 4 build types, and a separate toggle for NDEBUG:
debug: -O0 -DDEBUG (we add -DDEBUG)
debugoptimzed: -O2 -g
release: -O2
plain: (nothing)


I generally view meson as an upgrade in this respect, I guess mostly
because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
by me).. maybe making meson debug config use -O2 would bridge the gap?
  If so I have no problem with dropping -O0 and making debug config
also use -O2


Target without optimizations is good to track down issues triggered by
compiler optimizations, and to have faster builds.



Sounds good.

Also I think we should make -Db_ndebug=true the default for release builds.
!NDEBUG enables a lot more debugging code than just assertions.


That's already the case :)

The default value (line 29 of the root meson.build) is:

   b_ndebug=if-release

which means it will default to `true` if `buildtype=release`, and `false`
otherwise, while still being overridden if you manually set `b_ndebug`.


There needs to be something that's release with debug symbols, but
without _any_ other differences.  If build type assert handling differs
from release, debug symbols are less of a use for debugging issues
with release build.


Build type names could also be more descriptive, although something
like this seems overkill:
- release
- release_with_symbols
- release_with_symbols_and_asserts
- full_debug

Meson can help by outputting exact description of each build types,
e.g. when one doesn't specify one, or unrecognized one is provided.


- Eero
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Eric Engestrom
On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote:
> 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?

Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think
the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION
(the color suits me just fine :P), as both solutions satisfy the first
group but only the latter solution satisfies the 2nd group.

I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so
that it expresses the intent more clearly, with a prior patch to convert
Nine and other obvious !NDEBUG candidates, then, later on, some of the
EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-13 Thread Eric Engestrom
On Wednesday, 2018-12-12 19:45:06 -0500, Marek Olšák wrote:
> On Wed, Dec 12, 2018 at 7:35 PM Rob Clark  wrote:
> 
> > On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
> > >
> > > Quoting Rob Clark (2018-12-12 15:52:47)
> > > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker 
> > wrote:
> > > > >
> > > > > 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 guess my use cases for !=release builds are:
> > > >
> > > > + I want all the expensive checking because I'm not in it to win the
> > > >   deqp/piglit fps race
> > > > + I want debug syms for profiling and/or valgrind, but otherwise
> > > >   want something close to a release build but with debug syms
> > > >
> > > >
> > > > That said, I can get behind replacing DEBUG with !NDEBUG or
> > > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> > > >
> > > >
> > > > BR,
> > > > -R
> > >
> > > I guess I should have covered that:
> > >
> > > autotools had effectively two build types "debug" and "not debug",
> > "debug" set
> > > "-DDEBUG -g -O2", "not debug" set -DNDEBUG
> > >
> > > Meson has 4 build types, and a separate toggle for NDEBUG:
> > > debug: -O0 -DDEBUG (we add -DDEBUG)
> > > debugoptimzed: -O2 -g
> > > release: -O2
> > > plain: (nothing)
> >
> > I generally view meson as an upgrade in this respect, I guess mostly
> > because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
> > by me).. maybe making meson debug config use -O2 would bridge the gap?
> >  If so I have no problem with dropping -O0 and making debug config
> > also use -O2
> >
> 
> Sounds good.
> 
> Also I think we should make -Db_ndebug=true the default for release builds.
> !NDEBUG enables a lot more debugging code than just assertions.

That's already the case :)

The default value (line 29 of the root meson.build) is:

  b_ndebug=if-release

which means it will default to `true` if `buildtype=release`, and `false`
otherwise, while still being overridden if you manually set `b_ndebug`.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Ian Romanick
On 12/12/18 4:13 PM, Dylan Baker wrote:
> Quoting Rob Clark (2018-12-12 15:52:47)
>> On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker  wrote:
>>>
>>> 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 guess my use cases for !=release builds are:
>>
>> + I want all the expensive checking because I'm not in it to win the
>>   deqp/piglit fps race
>> + I want debug syms for profiling and/or valgrind, but otherwise
>>   want something close to a release build but with debug syms
>>
>>
>> That said, I can get behind replacing DEBUG with !NDEBUG or
>> EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
>>
>>
>> BR,
>> -R
> 
> I guess I should have covered that:
> 
> autotools had effectively two build types "debug" and "not debug", "debug" set
> "-DDEBUG -g -O2", "not debug" set -DNDEBUG
> 
> Meson has 4 build types, and a separate toggle for NDEBUG:
> debug: -O0 -DDEBUG (we add -DDEBUG)
> debugoptimzed: -O2 -g
> release: -O2
> plain: (nothing)

About 5 minutes into using meson, I started using plain builds with my
own flags set.  I can configure it *exactly* the way I want without
bothering or being bothered by anyone.  I haven't looked back since.

> Meson doesn't define NDEBUG by default, so if you want to turn off asserts you
> need to add -Db_ndebug=true
> 
> autotools debug is roughly between meson's debugoptimized and debug, while
> autotools non-debug corresponds to meson's plain buildtype.
> 
> Dylan
> 
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Jason Ekstrand
On Wed, Dec 12, 2018 at 6:45 PM Marek Olšák  wrote:

> On Wed, Dec 12, 2018 at 7:35 PM Rob Clark  wrote:
>
>> On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
>> >
>> > Quoting Rob Clark (2018-12-12 15:52:47)
>> > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker 
>> wrote:
>> > > >
>> > > > 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 guess my use cases for !=release builds are:
>> > >
>> > > + I want all the expensive checking because I'm not in it to win the
>> > >   deqp/piglit fps race
>> > > + I want debug syms for profiling and/or valgrind, but otherwise
>> > >   want something close to a release build but with debug syms
>> > >
>> > >
>> > > That said, I can get behind replacing DEBUG with !NDEBUG or
>> > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
>> > >
>> > >
>> > > BR,
>> > > -R
>> >
>> > I guess I should have covered that:
>> >
>> > autotools had effectively two build types "debug" and "not debug",
>> "debug" set
>> > "-DDEBUG -g -O2", "not debug" set -DNDEBUG
>> >
>> > Meson has 4 build types, and a separate toggle for NDEBUG:
>> > debug: -O0 -DDEBUG (we add -DDEBUG)
>> > debugoptimzed: -O2 -g
>> > release: -O2
>> > plain: (nothing)
>>
>> I generally view meson as an upgrade in this respect, I guess mostly
>> because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
>> by me).. maybe making meson debug config use -O2 would bridge the gap?
>>  If so I have no problem with dropping -O0 and making debug config
>> also use -O2
>>
>
> Sounds good.
>
> Also I think we should make -Db_ndebug=true the default for release
> builds. !NDEBUG enables a lot more debugging code than just assertions.
>

Agreed.  In any driver using NIR, the assertions enabled by !NDEBUG are
very expensive.

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Marek Olšák
On Wed, Dec 12, 2018 at 7:35 PM Rob Clark  wrote:

> On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
> >
> > Quoting Rob Clark (2018-12-12 15:52:47)
> > > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker 
> wrote:
> > > >
> > > > 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 guess my use cases for !=release builds are:
> > >
> > > + I want all the expensive checking because I'm not in it to win the
> > >   deqp/piglit fps race
> > > + I want debug syms for profiling and/or valgrind, but otherwise
> > >   want something close to a release build but with debug syms
> > >
> > >
> > > That said, I can get behind replacing DEBUG with !NDEBUG or
> > > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> > >
> > >
> > > BR,
> > > -R
> >
> > I guess I should have covered that:
> >
> > autotools had effectively two build types "debug" and "not debug",
> "debug" set
> > "-DDEBUG -g -O2", "not debug" set -DNDEBUG
> >
> > Meson has 4 build types, and a separate toggle for NDEBUG:
> > debug: -O0 -DDEBUG (we add -DDEBUG)
> > debugoptimzed: -O2 -g
> > release: -O2
> > plain: (nothing)
>
> I generally view meson as an upgrade in this respect, I guess mostly
> because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
> by me).. maybe making meson debug config use -O2 would bridge the gap?
>  If so I have no problem with dropping -O0 and making debug config
> also use -O2
>

Sounds good.

Also I think we should make -Db_ndebug=true the default for release builds.
!NDEBUG enables a lot more debugging code than just assertions.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Rob Clark
On Wed, Dec 12, 2018 at 7:14 PM Dylan Baker  wrote:
>
> Quoting Rob Clark (2018-12-12 15:52:47)
> > On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker  wrote:
> > >
> > > 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 guess my use cases for !=release builds are:
> >
> > + I want all the expensive checking because I'm not in it to win the
> >   deqp/piglit fps race
> > + I want debug syms for profiling and/or valgrind, but otherwise
> >   want something close to a release build but with debug syms
> >
> >
> > That said, I can get behind replacing DEBUG with !NDEBUG or
> > EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> >
> >
> > BR,
> > -R
>
> I guess I should have covered that:
>
> autotools had effectively two build types "debug" and "not debug", "debug" set
> "-DDEBUG -g -O2", "not debug" set -DNDEBUG
>
> Meson has 4 build types, and a separate toggle for NDEBUG:
> debug: -O0 -DDEBUG (we add -DDEBUG)
> debugoptimzed: -O2 -g
> release: -O2
> plain: (nothing)

I generally view meson as an upgrade in this respect, I guess mostly
because I have no use for '-DDEBUG -g -O2' (ie. the -O0 equiv is fine
by me).. maybe making meson debug config use -O2 would bridge the gap?
 If so I have no problem with dropping -O0 and making debug config
also use -O2

BR.
-R

>
> Meson doesn't define NDEBUG by default, so if you want to turn off asserts you
> need to add -Db_ndebug=true
>
> autotools debug is roughly between meson's debugoptimized and debug, while
> autotools non-debug corresponds to meson's plain buildtype.
>
> Dylan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Dylan Baker
Quoting Rob Clark (2018-12-12 15:52:47)
> On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker  wrote:
> >
> > 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 guess my use cases for !=release builds are:
> 
> + I want all the expensive checking because I'm not in it to win the
>   deqp/piglit fps race
> + I want debug syms for profiling and/or valgrind, but otherwise
>   want something close to a release build but with debug syms
> 
> 
> That said, I can get behind replacing DEBUG with !NDEBUG or
> EXPENSIVE_DEBUG or whatever permutation of that color folks prefer
> 
> 
> BR,
> -R

I guess I should have covered that:

autotools had effectively two build types "debug" and "not debug", "debug" set
"-DDEBUG -g -O2", "not debug" set -DNDEBUG

Meson has 4 build types, and a separate toggle for NDEBUG:
debug: -O0 -DDEBUG (we add -DDEBUG)
debugoptimzed: -O2 -g
release: -O2
plain: (nothing)

Meson doesn't define NDEBUG by default, so if you want to turn off asserts you
need to add -Db_ndebug=true

autotools debug is roughly between meson's debugoptimized and debug, while
autotools non-debug corresponds to meson's plain buildtype.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Rob Clark
On Wed, Dec 12, 2018 at 6:25 PM Dylan Baker  wrote:
>
> 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 guess my use cases for !=release builds are:

+ I want all the expensive checking because I'm not in it to win the
  deqp/piglit fps race
+ I want debug syms for profiling and/or valgrind, but otherwise
  want something close to a release build but with debug syms


That said, I can get behind replacing DEBUG with !NDEBUG or
EXPENSIVE_DEBUG or whatever permutation of that color folks prefer


BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2018-12-12 Thread Dylan Baker
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?

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev