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

[David Blaikie via llvm-dev]

On Fri, Apr 10, 2020 at 12:35 PM Fāng-ruì Sòng <maskray at google.com> wrote:

> 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...
>

I'm not sure that "many NDEBUG uses should actually be
LLVM_ENABLE_ABI_BREAKING_CHECKS" - I would guess that most NDEBUG uses are
correctly non-abi breaking, but those that are abi-breaking can/should be
changed/fixed. Not all of those 700+ are abi breaking, probably most
aren't. And the folks that introduced LLVM_ENABLE_ABI_BREAKING_CHECKS and
other folks who've found that mode valuable have made fixes/continued to
maintain the feature I think.

Perhaps not - perhaps the feature has bitrotted to the point of
uselessness, in which case if no one's interested in it, it could be
removed. I don't believe that's the case, though - just a guess.


>
>
>>
>>
>
>>> >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/68e76873/attachment.html>