llvm dev - Apr 2020 - [RFC] Usage of NDEBUG as a guard for non-assert debug code

On 2020-04-10, Michael Kruse via llvm-dev wrote:
>#ifndef NDEBUG in the LLVM source and assert() are at least somewhat
>linked. For instance, consider
>https://github.com/llvm/llvm-project/blob/8423a6f36386aabced2a367c0ea53487901d31ca/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp#L2668
>
>The #ifndef NDEBUG is used to guard the value that checked in an
>assert() later. Only enabling assert() will cause the build to fail
>because the value is not defined. Another example is a loop only for
>the assert, eg.
>
>    #ifndef NDEBUG
>    for (auto &Val : Container)
>      assert(Val && "all");
>    #endif
>
>where the loop would loop over nothing with assertions disable. We
>cannot have a 'change all 'NDEBUG to LLVM_NO_DEBUG_CHECKS'. Even
if we
>manage to leave all NDEBUG that are linked to some use of assert(), it
>doubles the number of configurations that might break in some
>configuration such as IndVarSimplify. I understand NDEBUG as `remove
>all the code only useful for developers`, independent of whether we
>also want debug symbols.
>
>I'd find it more useful to discuss what should NOT be covered under
>the blanket term LLVM_ENABLE_ASSERTIONS. Example from the past are
>LLVM_ENABLE_STATS and LLVM_ENABLE_DUMP that once was also using
>NDEBUG. 
+1 for bring up this topic.


The code base sometimes uses #ifdef NDEBUG to guard the definitions of
some struct/class members and functions.

This means there are inherent ABI incompatibility between non-NDEBUG and
NDEBUG object files. I hoped that mixing object files could work(at
least in some configurations.  Also note that having different
definitions of inline functions leads to ODR violation) but I think this
cannot be changed any more.
>As was mentioned, Support/Debug.h might be another candidate.
>But IMHO the compile-time/execution-time/code-size impact of these are
>too small to justify the increase combinatorial complexity of
>configuration. At the last LLVM DevMtg we actually discussed how to
>*reduce* the number of independent configuration parameters.
>
>Michael

On Fri, Apr 10, 2020 at 10:15 AM Fangrui Song via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On 2020-04-10, Michael Kruse via llvm-dev wrote:
> >#ifndef NDEBUG in the LLVM source and assert() are at least somewhat
> >linked. For instance, consider
> >
>
https://github.com/llvm/llvm-project/blob/8423a6f36386aabced2a367c0ea53487901d31ca/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp#L2668
> >
> >The #ifndef NDEBUG is used to guard the value that checked in an
> >assert() later. Only enabling assert() will cause the build to fail
> >because the value is not defined. Another example is a loop only for
> >the assert, eg.
> >
> >    #ifndef NDEBUG
> >    for (auto &Val : Container)
> >      assert(Val && "all");
> >    #endif
> >
> >where the loop would loop over nothing with assertions disable. We
> >cannot have a 'change all 'NDEBUG to LLVM_NO_DEBUG_CHECKS'.
Even if we
> >manage to leave all NDEBUG that are linked to some use of assert(), it
> >doubles the number of configurations that might break in some
> >configuration such as IndVarSimplify. I understand NDEBUG as `remove
> >all the code only useful for developers`, independent of whether we
> >also want debug symbols.
> >
> >I'd find it more useful to discuss what should NOT be covered under
> >the blanket term LLVM_ENABLE_ASSERTIONS. Example from the past are
> >LLVM_ENABLE_STATS and LLVM_ENABLE_DUMP that once was also using
> >NDEBUG.
>
> +1 for bring up this topic.
>
>
> The code base sometimes uses #ifdef NDEBUG to guard the definitions of
> some struct/class members and functions.
>
> This means there are inherent ABI incompatibility between non-NDEBUG and
> NDEBUG object files. I hoped that mixing object files could work(at
> least in some configurations.  Also note that having different
> definitions of inline functions leads to ODR violation) but I think this
> cannot be changed any more.
>
Not sure I'm following - what "cannot be changed anymore"? If
there are ABI
changing uses of NDEBUG they can and should be changed to
LLVM_ENABLE_ABI_BREAKING_CHECKS.

>
> >As was mentioned, Support/Debug.h might be another candidate.
> >But IMHO the compile-time/execution-time/code-size impact of these are
> >too small to justify the increase combinatorial complexity of
> >configuration. At the last LLVM DevMtg we actually discussed how to
> >*reduce* the number of independent configuration parameters.
> >
> >Michael
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200410/08fa9576/attachment.html>

On Fri, Apr 10, 2020 at 12:26 PM David Blaikie <dblaikie at gmail.com>
wrote:
>
>
> On Fri, Apr 10, 2020 at 10:15 AM Fangrui Song via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On 2020-04-10, Michael Kruse via llvm-dev wrote:
>> >#ifndef NDEBUG in the LLVM source and assert() are at least
somewhat
>> >linked. For instance, consider
>> >
>>
https://github.com/llvm/llvm-project/blob/8423a6f36386aabced2a367c0ea53487901d31ca/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp#L2668
>> >
>> >The #ifndef NDEBUG is used to guard the value that checked in an
>> >assert() later. Only enabling assert() will cause the build to fail
>> >because the value is not defined. Another example is a loop only
for
>> >the assert, eg.
>> >
>> >    #ifndef NDEBUG
>> >    for (auto &Val : Container)
>> >      assert(Val && "all");
>> >    #endif
>> >
>> >where the loop would loop over nothing with assertions disable. We
>> >cannot have a 'change all 'NDEBUG to
LLVM_NO_DEBUG_CHECKS'. Even if we
>> >manage to leave all NDEBUG that are linked to some use of assert(),
it
>> >doubles the number of configurations that might break in some
>> >configuration such as IndVarSimplify. I understand NDEBUG as
`remove
>> >all the code only useful for developers`, independent of whether we
>> >also want debug symbols.
>> >
>> >I'd find it more useful to discuss what should NOT be covered
under
>> >the blanket term LLVM_ENABLE_ASSERTIONS. Example from the past are
>> >LLVM_ENABLE_STATS and LLVM_ENABLE_DUMP that once was also using
>> >NDEBUG.
>>
>> +1 for bring up this topic.
>>
>>
>> The code base sometimes uses #ifdef NDEBUG to guard the definitions of
>> some struct/class members and functions.
>>
>> This means there are inherent ABI incompatibility between non-NDEBUG
and
>> NDEBUG object files. I hoped that mixing object files could work(at
>> least in some configurations.  Also note that having different
>> definitions of inline functions leads to ODR violation) but I think
this
>> cannot be changed any more.
>>
>
> Not sure I'm following - what "cannot be changed anymore"? If
there are
> ABI changing uses of NDEBUG they can and should be changed to
> LLVM_ENABLE_ABI_BREAKING_CHECKS.
>
I agree that many NDEBUG uses are actually LLVM_ENABLE_ABI_BREAKING_CHECKS.
By "cannot be changed anymore" I mean there are so many NDEBUG (700+
files),
I am not sure we can still make mixing non-NDEBUG and NDEBUG object files
work...

>
>
>> >As was mentioned, Support/Debug.h might be another candidate.
>> >But IMHO the compile-time/execution-time/code-size impact of these
are
>> >too small to justify the increase combinatorial complexity of
>> >configuration. At the last LLVM DevMtg we actually discussed how to
>> >*reduce* the number of independent configuration parameters.
>> >
>> >Michael
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200410/387e6e69/attachment.html>