(original) (raw)


On Apr 10, 2017, at 12:37 PM, Matthias Braun <mbraun@apple.com> wrote:

The situation is not consistent. Yes there are several places where we have the #if in the headers however there are far more cases where it is not. Some points here:

- This whole LLVM\_DUMP\_FUNCTION/LLVM\_ENABLE\_DUMP is about enabling the linker to strip (or not strip) the dumping function in release (debug) builds.
- For this it doesn't matter whether you have a declaration in the header or not, so it seems we standardized on not having it there.
- Things are 100% consistent so we sometimes have #ifs anyway.
- In case of templates we not only have the declaration but also an implementation in the header and need the #if there
- A similar problem arises in cases where the dump function was declared virtual and ends up in the vtable
- If you ask me then we shouldn't have LLVM\_ENABLE\_DUMP and just rely on NDEBUG to keep things simple... (We're in this strange state anyway where LLVM\_ENABLE\_DEBUG isn't even exposed as a cmake option).
- Either way, putting LLVM\_ENABLE\_DUMP into config.h would make the status-quo more consistent.

Using the config.h instead of the NDEBUG makes it robust against client using NDEBUG differently from what LLVM was built with. This seems really better to me.

Mehdi




- Matthias

On Apr 10, 2017, at 12:17 PM, Chris Bieneman <beanz@apple.com> wrote:

Presently several of our headers have definitions like:

#if !defined(NDEBUG) || defined(LLVM\_ENABLE\_DUMP)
void dump() const;
#endif

Would it make sense to modify the build system to define LLVM\_ENABLE\_DUMP in config.h on debug builds?

Then we could wrap dump methods just based on LLVM\_ENABLE\_DUMP instead of two variables.

-Chris

On Apr 10, 2017, at 1:34 PM, Robinson, Paul via llvm-dev <llvm-dev@lists.llvm.org> wrote:



-----Original Message-----
From: llvm-dev \[mailto:llvm-dev-bounces@lists.llvm.org\] On Behalf Of Mehdi
Amini via llvm-dev
Sent: Sunday, April 09, 2017 2:26 PM
To: Matthias Braun
Cc: llvm-dev@lists.llvm.org; jingu@codeplay.com
Subject: Re: \[llvm-dev\] Question about LLVM Building Error with "-
DLLVM\_ENABLE\_DUMP" and "RelWithDebInfo"


On Apr 7, 2017, at 4:45 PM, Matthias Braun via llvm-dev
dev@lists.llvm.org> wrote:

I think the idea is to keep NDEBUG out of headers when possible. So I
think this should better be something like:

-#ifndef NDEBUG
void dumpUses(unsigned RegNo) const;
-#endif

to be inline with various other dumpers (like MachineInstr::dump(),
Pass::dump(), …)

I’m fine with leaving methods there, but we need to be able to compile-out
fields in structure.

Hmmm seems to me this has come up in the past, and somebody pointed out
that it prevents building a debug-mode front-end against a release-mode LLVM.
(Why is that a valid use-case? If I have an out-of-tree front end, and
especially one with a different license, I might well prefer to download
only LLVM releases rather than keep up-to-date with a live tree that I
build myself. IIRC we do not provide debug-mode downloads, therefore
anything that affects struct size/layout will break this use-case.)
--paulr


We already have ABI\_BREAKING\_CHECKS for instance to this end, the naming
isn’t completely in line with LLVM\_ENABLE\_DUMP but could be unified.


Mehdi



If that works for you please submit a patch to phabricator as described
in http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch

- Matthias

On Apr 6, 2017, at 7:38 AM, jingu@codeplay.com via llvm-dev
dev@lists.llvm.org> wrote:

Hi All,

I have tried to build llvm tip as following:

cmake -DCMAKE\_CXX\_FLAGS:STRING="-DLLVM\_ENABLE\_DUMP" -
DCMAKE\_BUILD\_TYPE=RelWithDebInfo ../llvm

After running 'make', I have got error messages like below.

llvm/lib/CodeGen/MachineRegisterInfo.cpp:462:67: error: no ‘void
llvm::MachineRegisterInfo::dumpUses(unsigned int) const’ member function
declared in class ‘llvm::MachineRegisterInfo’

llvm/lib/CodeGen/MachineScheduler.cpp:2331:57: error: no ‘void
llvm::SchedBoundary::dumpScheduledState()’ member function declared in
class ‘llvm::SchedBoundary’

...

It seems the "defined(LLVM\_ENABLE\_DUMP)" is needed on several
locations. How do you think about it? I have attached the diff file about
the locations for reference. If I missed something, please let me know.

Thanks,

JinGu Kang

\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
LLVM Developers mailing list
llvm-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
LLVM Developers mailing list
llvm-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
LLVM Developers mailing list
llvm-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
LLVM Developers mailing list
llvm-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev