[Mesa-dev,2/2] configure.ac: add --enable-assertions

Submitted by Marek Olšák on April 13, 2015, 8:06 p.m.

Details

Message ID 1428955610-32350-2-git-send-email-maraeo@gmail.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Marek Olšák April 13, 2015, 8:06 p.m.
From: Marek Olšák <marek.olsak@amd.com>

---
 configure.ac | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 6ccf3b4..f5eeb7d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -402,6 +402,13 @@  AC_ARG_ENABLE([debug],
     [enable_debug="$enableval"],
     [enable_debug=no]
 )
+AC_ARG_ENABLE([assertions],
+    [AS_HELP_STRING([--enable-assertions],
+        [add debug definitions to enable assertions and other debug checks @<:@default=disabled@:>@])],
+    [enable_assertions="$enableval"],
+    [enable_assertions=no]
+)
+
 if test "x$enable_debug" = xyes; then
     DEFINES="$DEFINES -DDEBUG"
     if test "x$GCC" = xyes; then
@@ -420,8 +427,10 @@  if test "x$enable_debug" = xyes; then
             CXXFLAGS="$CXXFLAGS -O0"
         fi
     fi
+elif test "x$enable_assertions" = xyes; then
+    DEFINES="$DEFINES -DDEBUG"
 else
-   DEFINES="$DEFINES -DNDEBUG"
+    DEFINES="$DEFINES -DNDEBUG"
 fi
 
 dnl

Comments

Hi Marek

On 13 April 2015 at 21:06, Marek Olšák <maraeo@gmail.com> wrote:
> From: Marek Olšák <marek.olsak@amd.com>
>
> ---
>  configure.ac | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 6ccf3b4..f5eeb7d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -402,6 +402,13 @@ AC_ARG_ENABLE([debug],
>      [enable_debug="$enableval"],
>      [enable_debug=no]
>  )
> +AC_ARG_ENABLE([assertions],
> +    [AS_HELP_STRING([--enable-assertions],
> +        [add debug definitions to enable assertions and other debug checks @<:@default=disabled@:>@])],
Can we keep this only for asserts (NDEBUG) as the name suggests ?
Otherwise things might get bit too messy when combining with
--enable-debug. Especially since most places of mesa reply on DEBUG,
while the asserts definition depends on NDEBUG.

Thanks
Emil
If --enable-debug is used, --enable-assertions is ignored.
--enable-assertions is basically a weaker version of --enable-debug.

There are 2 assert definitions in Mesa:
- The one that is enabled if DEBUG is defined (gallium only).
- The one that is enabled if NDEBUG is not defined (the standard version).

Both conditions must be met for assertions to be enabled everywhere.

If somebody wants to remove the gallium assert in favor of the
standard one, then be it, but that's beyond the scope of this patch.

Marek

On Tue, Apr 14, 2015 at 2:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi Marek
>
> On 13 April 2015 at 21:06, Marek Olšák <maraeo@gmail.com> wrote:
>> From: Marek Olšák <marek.olsak@amd.com>
>>
>> ---
>>  configure.ac | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 6ccf3b4..f5eeb7d 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -402,6 +402,13 @@ AC_ARG_ENABLE([debug],
>>      [enable_debug="$enableval"],
>>      [enable_debug=no]
>>  )
>> +AC_ARG_ENABLE([assertions],
>> +    [AS_HELP_STRING([--enable-assertions],
>> +        [add debug definitions to enable assertions and other debug checks @<:@default=disabled@:>@])],
> Can we keep this only for asserts (NDEBUG) as the name suggests ?
> Otherwise things might get bit too messy when combining with
> --enable-debug. Especially since most places of mesa reply on DEBUG,
> while the asserts definition depends on NDEBUG.
>
> Thanks
> Emil
On 14 April 2015 at 13:27, Marek Olšák <maraeo@gmail.com> wrote:
> If --enable-debug is used, --enable-assertions is ignored.
> --enable-assertions is basically a weaker version of --enable-debug.
>
Doesn't this make things more counter-intuitive ? I'm guilty of making
--enable-debug a bit messy (and I'm ok with reverting it), but please
don't take it as a role model.

> There are 2 assert definitions in Mesa:
> - The one that is enabled if DEBUG is defined (gallium only).
Hmm nice. Seem that we can wrap it in ifndef NDEBUG ... else ... endif
without breaking existing users.

> - The one that is enabled if NDEBUG is not defined (the standard version).
>
> Both conditions must be met for assertions to be enabled everywhere.
>
> If somebody wants to remove the gallium assert in favor of the
> standard one, then be it, but that's beyond the scope of this patch.
>
Suspecting that removing it might be out of the question, although the
above suggestion should just work for everyone. What do you think ?


-Emil


> Marek
>
> On Tue, Apr 14, 2015 at 2:13 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> Hi Marek
>>
>> On 13 April 2015 at 21:06, Marek Olšák <maraeo@gmail.com> wrote:
>>> From: Marek Olšák <marek.olsak@amd.com>
>>>
>>> ---
>>>  configure.ac | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 6ccf3b4..f5eeb7d 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -402,6 +402,13 @@ AC_ARG_ENABLE([debug],
>>>      [enable_debug="$enableval"],
>>>      [enable_debug=no]
>>>  )
>>> +AC_ARG_ENABLE([assertions],
>>> +    [AS_HELP_STRING([--enable-assertions],
>>> +        [add debug definitions to enable assertions and other debug checks @<:@default=disabled@:>@])],
>> Can we keep this only for asserts (NDEBUG) as the name suggests ?
>> Otherwise things might get bit too messy when combining with
>> --enable-debug. Especially since most places of mesa reply on DEBUG,
>> while the asserts definition depends on NDEBUG.
>>
>> Thanks
>> Emil
On Tue, Apr 14, 2015 at 3:12 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 14 April 2015 at 13:27, Marek Olšák <maraeo@gmail.com> wrote:
>> If --enable-debug is used, --enable-assertions is ignored.
>> --enable-assertions is basically a weaker version of --enable-debug.
>>
> Doesn't this make things more counter-intuitive ? I'm guilty of making
> --enable-debug a bit messy (and I'm ok with reverting it), but please
> don't take it as a role model.
>
>> There are 2 assert definitions in Mesa:
>> - The one that is enabled if DEBUG is defined (gallium only).
> Hmm nice. Seem that we can wrap it in ifndef NDEBUG ... else ... endif
> without breaking existing users.
>
>> - The one that is enabled if NDEBUG is not defined (the standard version).
>>
>> Both conditions must be met for assertions to be enabled everywhere.
>>
>> If somebody wants to remove the gallium assert in favor of the
>> standard one, then be it, but that's beyond the scope of this patch.
>>
> Suspecting that removing it might be out of the question, although the
> above suggestion should just work for everyone. What do you think ?

I wouldn't like to lose other debug checking that depends on DEBUG.

BTW, I no longer need this patch. It seems --enable-debug doesn't
disable optimizations by default, so I will use that. This issue is
closed.

Marek