<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Dec 13, 2018 at 5:06 AM Eric Engestrom <<a href="mailto:eric.engestrom@intel.com">eric.engestrom@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wednesday, 2018-12-12 15:24:25 -0800, Dylan Baker wrote:<br>
> In the autotools discussion I've come to realize that we also need to talk about<br>
> the -DDEBUG guard. It seems that there are two different uses, and thus two<br>
> different asks about it:<br>
> <br>
> - Nine (and RadeonSI?) use -DDEBUG to hide generic debugging<br>
> - NIR and Intel (at least) use -DDEBUG to hide really expensive checks that are<br>
>   useful, but necessarily tank performance.<br>
> <br>
> The first group would like -DDEBUG in debugoptimized builds, the second<br>
> obviously doesn't.<br>
> <br>
> Is the right solution to move the first group being !NDEBUG, or would it be<br>
> better to split DEBUG into two different defines such as DEBUG_MESSAGES and<br>
> EXPENSIVE_VALIDATION (paint the bikeshed whatever color you like), with the<br>
> first for both debug and debugoptimized and the second only in debug builds?<br>
<br>
Replacing DEBUG with !NDEBUG is obviously trivially simpler, but I think<br>
the right thing would be to split it into !NDEBUG and EXPENSIVE_VALIDATION<br>
(the color suits me just fine :P), as both solutions satisfy the first<br>
group but only the latter solution satisfies the 2nd group.<br>
<br>
I think a first pass might be to simply s/DEBUG/EXPENSIVE_VALIDATION/ so<br>
that it expresses the intent more clearly, with a prior patch to convert<br>
Nine and other obvious !NDEBUG candidates, then, later on, some of the<br>
EXPENSIVE_VALIDATION can be promoted to !NDEBUG on a case-by-case basis.<br></blockquote><div><br></div><div>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.<br></div><div><br></div><div>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.<br></div><div><br></div><div>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.</div><div><br></div><div>--Jason<br></div></div></div>