(original) (raw)


On Apr 10, 2017, at 2: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).

See: http://bugs.llvm.org/show\_bug.cgi?id=32283

I might just have to fix that this week now that you've brought it up :-)

-Chris

- Either way, putting LLVM\_ENABLE\_DUMP into config.h would make the status-quo more consistent.

- 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