(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
- MatthiasOn 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;#endifWould 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.-ChrisOn 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-devdev@lists.llvm.org> wrote:think this should better be something like:
I think the idea is to keep NDEBUG out of headers when possible. So IPass::dump(), …)
-#ifndef NDEBUG
void dumpUses(unsigned RegNo) const;
-#endif
to be inline with various other dumpers (like MachineInstr::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.
—
Mehdiin http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch
If that works for you please submit a patch to phabricator as describeddev@lists.llvm.org> wrote:
- MatthiasOn Apr 6, 2017, at 7:38 AM, jingu@codeplay.com via llvm-devDCMAKE\_BUILD\_TYPE=RelWithDebInfo ../llvm
Hi All,
I have tried to build llvm tip as following:
cmake -DCMAKE\_CXX\_FLAGS:STRING="-DLLVM\_ENABLE\_DUMP" -llvm::MachineRegisterInfo::dumpUses(unsigned int) const’ member function
After running 'make', I have got error messages like below.
llvm/lib/CodeGen/MachineRegisterInfo.cpp:462:67: error: no ‘void
declared in class ‘llvm::MachineRegisterInfo’llvm::SchedBoundary::dumpScheduledState()’ member function declared in
llvm/lib/CodeGen/MachineScheduler.cpp:2331:57: error: no ‘void
class ‘llvm::SchedBoundary’locations. How do you think about it? I have attached the diff file about
...
It seems the "defined(LLVM\_ENABLE\_DUMP)" is needed on several
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