llvm dev - Apr 2017 - Question about LLVM Building Error with "-DLLVM_ENABLE_DUMP" and "RelWithDebInfo"

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 at
lists.llvm.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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 <llvm-
>> dev at 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 at codeplay.com via llvm-dev
<llvm-
>> dev at 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
>>>> 
>>>>
<dump.diff>_______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/80790ecf/attachment.html>

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.

- Matthias
> On Apr 10, 2017, at 12:17 PM, Chris Bieneman <beanz at 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
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> 
>> 
>>> -----Original Message-----
>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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
<llvm-
>>> dev at lists.llvm.org <mailto:dev at 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
<http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch>
>>>> 
>>>> - Matthias
>>>> 
>>>>> On Apr 6, 2017, at 7:38 AM, jingu at codeplay.com
<mailto:jingu at codeplay.com> via llvm-dev <llvm-
>>> dev at lists.llvm.org <mailto:dev at 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
>>>>> 
>>>>>
<dump.diff>_______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/69e0a2ab/attachment.html>

> On Apr 10, 2017, at 12:37 PM, Matthias Braun <mbraun at 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.
This should read:
"...  so it seems we standardized on not having the #if 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.
> 
> - Matthias
> 
>> On Apr 10, 2017, at 12:17 PM, Chris Bieneman <beanz at apple.com
<mailto:beanz at 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 at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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
<llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
<http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch>
>>>>> 
>>>>> - Matthias
>>>>> 
>>>>>> On Apr 6, 2017, at 7:38 AM, jingu at codeplay.com
<mailto:jingu at codeplay.com> via llvm-dev <llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
>>>>>> 
>>>>>>
<dump.diff>_______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>>> 
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/85403a82/attachment-0001.html>

> On Apr 10, 2017, at 12:37 PM, Matthias Braun <mbraun at 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 at apple.com
<mailto:beanz at 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 at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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
<llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
<http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch>
>>>>> 
>>>>> - Matthias
>>>>> 
>>>>>> On Apr 6, 2017, at 7:38 AM, jingu at codeplay.com
<mailto:jingu at codeplay.com> via llvm-dev <llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
>>>>>> 
>>>>>>
<dump.diff>_______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>>> 
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/dc091eb0/attachment.html>

> On Apr 10, 2017, at 2:37 PM, Matthias Braun <mbraun at 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
<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 at apple.com
<mailto:beanz at 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 at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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
<llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
<http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch>
>>>>> 
>>>>> - Matthias
>>>>> 
>>>>>> On Apr 6, 2017, at 7:38 AM, jingu at codeplay.com
<mailto:jingu at codeplay.com> via llvm-dev <llvm-
>>>> dev at lists.llvm.org <mailto:dev at 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
>>>>>> 
>>>>>>
<dump.diff>_______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>>> 
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/fcab3e79/attachment.html>

> On Apr 10, 2017, at 12:17 PM, Chris Bieneman <beanz at 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.
I missed this email earlier, but that totally makes sense to me (ideally with a
link/runtime check like the ABI_BREAKING_CHECK one).

Thanks,

— 
Mehdi

>> On Apr 10, 2017, at 1:34 PM, Robinson, Paul via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> 
>> 
>>> -----Original Message-----
>>> From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at 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 at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>; jingu at codeplay.com <mailto:jingu at 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
<llvm-
>>> dev at lists.llvm.org <mailto:dev at 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
<http://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch>
>>>> 
>>>> - Matthias
>>>> 
>>>>> On Apr 6, 2017, at 7:38 AM, jingu at codeplay.com
<mailto:jingu at codeplay.com> via llvm-dev <llvm-
>>> dev at lists.llvm.org <mailto:dev at 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
>>>>> 
>>>>>
<dump.diff>_______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>> 
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://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/20170410/8952d9e1/attachment.html>