[llvm-commits] [llvm] r163339 - in /llvm/trunk/lib: CodeGen/ CodeGen/SelectionDAG/ Target/X86/

[Duncan Sands]

Hi Chandler,

On 07/09/12 22:12, Chandler Carruth wrote:
> On Fri, Sep 7, 2012 at 3:52 PM, Duncan Sands <baldrick at free.fr
> <mailto:baldrick at free.fr>> wrote:
>
>     On 07/09/12 20:36, Bill Wendling wrote:
>
>         On Sep 7, 2012, at 9:38 AM, Duncan Sands <baldrick at free.fr
>         <mailto:baldrick at free.fr>> wrote:
>
>             Hi Craig,
>
>             On 07/09/12 18:18, Craig Topper wrote:
>
>                 Wouldn't dead code stripping in the linker have removed these
>                 from a released build?
>
>
>             good question, but it seems not since I regularly use these when
>             debugging
>             release build problems on linux (usually buildbot crashes).  (I
>             don't have
>             time to look into why they aren't removed right now, sorry).  If it
>             was the
>             case that the linker removes them then Manman's patch wouldn't really be
>             needed :)
>
>         If you need them, then we should think of another method for removing
>         them that works for us but doesn't affect your stuff.
>
>         Manman, what do you think of having a flag, say REMOVE_DUMP, instead of
>         using NDEBUG?
>
>
>     how about removing them when doing a build with no debug symbols, i.e. rather
>     than conditioning on the presence of checks (NDEBUG), condition on the presence
>     of debug info.  If there is no debug info then even I am not going to be
>     calling these guys from the debugger, debugging is just too hard without debug
>     info!  (The buildbots I get to debug from time to time are built as release-
>     with-debug-info).
>
>
> I actually think NDEBUG makes a lot of sense here though... Debugging is also
> too hard without asserts! ;]

unfortunately you sometimes have to do it.

> What makes the most sense to me is this:
>
> 1) Have a specific #define which will always enable the dump methods. I'll call
> it "LLVM_ENABLE_DUMP".
> 2) If not defined in the build, default this flag based on NDEBUG -> if !NDEBUG
> #define it to 1.
>
> This would allow you to configure your buildbots with -DLLVM_ENABLE_DUMP Duncan
> if you wanted, or allow a normal dev to rebuild with that flag if there is a bug
> that cannot be reproduced with asserts enabled. It would keep these methods
> available for typical Release+Asserts builds. It would remove them from typical
> actual released binaries.

I agree that you don't want to conflate "have dump methods" with "have debug
symbols" or "checking is turned on" in the code base, i.e. I agree that there
should be something like LLVM_ENABLE_DUMP which turns this on or off rather
than using NDEBUG or whatever directly.

There remains the question of what the default logic for turning this on is.
I would rather turn it on whenever compiling with "-g".

The logic of this is: if you compile without -g then debugging is literally
impossible, and you won't be able to call "dump" from within the debugger
anyway.  On the other hand, if the user compiled LLVM with -g then they are
interested in debugging, thus things useful for debugging like "dump" should
be available.

Ciao, Duncan.