Dibyendu Majumdar via llvm-dev
2017-Sep-25 21:41 UTC
[llvm-dev] Errors linking with LLVM 5.0 - dump() missing
Hi Don, On 25 September 2017 at 22:37, Don Hinton <hintonda at gmail.com> wrote:> It'll work in release builds -- just rebuild llvm with LLVM_ENABLE_DUMP > enabled. >That assumes one has control over the LLVM build options.> On Mon, Sep 25, 2017 at 2:35 PM, Dibyendu Majumdar <mobile at majumdar.org.uk> > wrote: >> >> On 25 September 2017 at 22:29, Don Hinton <hintonda at gmail.com> wrote: >> > Thanks for reporting this. >> > >> > Looks like this one was missed -- the declaration should have been >> > #ifdef'd >> > away along with the definition. A quick grep indicates there are a >> > number >> > of them that need to be fixed. >> > >> > Here's the original commit: >> > >> > commit 88d207542b618ca6054b24491ddd67f8ca397540 >> > Author: Matthias Braun <matze at braunis.de> >> > Date: Sat Jan 28 02:02:38 2017 +0000 >> > >> > Cleanup dump() functions. >> > >> > We had various variants of defining dump() functions in LLVM. >> > Normalize >> > them (this should just consistently implement the things discussed >> > in >> > http://lists.llvm.org/pipermail/cfe-dev/2014-January/034323.html >> > >> > For reference: >> > - Public headers should just declare the dump() method but not use >> > LLVM_DUMP_METHOD or #if !defined(NDEBUG) || >> > defined(LLVM_ENABLE_DUMP) >> > - The definition of a dump method should look like this: >> > #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) >> > LLVM_DUMP_METHOD void MyClass::dump() { >> > // print stuff to dbgs()... >> > } >> > #endif >> > >> > git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at 293359 >> > 91177308-0d34-0410-b5e6-96231b3b80d8 >> > >> >> I would argue that removing dump() is the wrong thing to do even in >> Release builds. I am using LLVM as a JIT. In this use case, it is >> essential that one can inspect the IR at runtime. Perhaps in the AOT >> case dump() is not used in release versions but that is not the case >> in a JIT use case. >> >> Unless there is an equivalent alternative to dump() that is always >> available. >> >>Regards Dibyendu
Don Hinton via llvm-dev
2017-Sep-25 21:43 UTC
[llvm-dev] Errors linking with LLVM 5.0 - dump() missing
Or, instead of lobbying for llvm to always define dump(), you could lobby for it to enabled by default. On Mon, Sep 25, 2017 at 2:41 PM, Dibyendu Majumdar <mobile at majumdar.org.uk> wrote:> Hi Don, > > On 25 September 2017 at 22:37, Don Hinton <hintonda at gmail.com> wrote: > > It'll work in release builds -- just rebuild llvm with LLVM_ENABLE_DUMP > > enabled. > > > > That assumes one has control over the LLVM build options. > > > > On Mon, Sep 25, 2017 at 2:35 PM, Dibyendu Majumdar < > mobile at majumdar.org.uk> > > wrote: > >> > >> On 25 September 2017 at 22:29, Don Hinton <hintonda at gmail.com> wrote: > >> > Thanks for reporting this. > >> > > >> > Looks like this one was missed -- the declaration should have been > >> > #ifdef'd > >> > away along with the definition. A quick grep indicates there are a > >> > number > >> > of them that need to be fixed. > >> > > >> > Here's the original commit: > >> > > >> > commit 88d207542b618ca6054b24491ddd67f8ca397540 > >> > Author: Matthias Braun <matze at braunis.de> > >> > Date: Sat Jan 28 02:02:38 2017 +0000 > >> > > >> > Cleanup dump() functions. > >> > > >> > We had various variants of defining dump() functions in LLVM. > >> > Normalize > >> > them (this should just consistently implement the things discussed > >> > in > >> > http://lists.llvm.org/pipermail/cfe-dev/2014-January/034323.html > >> > > >> > For reference: > >> > - Public headers should just declare the dump() method but not use > >> > LLVM_DUMP_METHOD or #if !defined(NDEBUG) || > >> > defined(LLVM_ENABLE_DUMP) > >> > - The definition of a dump method should look like this: > >> > #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) > >> > LLVM_DUMP_METHOD void MyClass::dump() { > >> > // print stuff to dbgs()... > >> > } > >> > #endif > >> > > >> > git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at 293359 > >> > 91177308-0d34-0410-b5e6-96231b3b80d8 > >> > > >> > >> I would argue that removing dump() is the wrong thing to do even in > >> Release builds. I am using LLVM as a JIT. In this use case, it is > >> essential that one can inspect the IR at runtime. Perhaps in the AOT > >> case dump() is not used in release versions but that is not the case > >> in a JIT use case. > >> > >> Unless there is an equivalent alternative to dump() that is always > >> available. > >> > >> > > Regards > Dibyendu >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170925/fd88e47a/attachment.html>
Dibyendu Majumdar via llvm-dev
2017-Sep-25 21:45 UTC
[llvm-dev] Errors linking with LLVM 5.0 - dump() missing
Hi Don, On 25 September 2017 at 22:43, Don Hinton <hintonda at gmail.com> wrote:> Or, instead of lobbying for llvm to always define dump(), you could lobby > for it to enabled by default. >I was about to suggest the same thing - i.e. should not the default be to maintain compatibility with earlier releases, and the new build option to change the default?>> On 25 September 2017 at 22:37, Don Hinton <hintonda at gmail.com> wrote: >> > It'll work in release builds -- just rebuild llvm with LLVM_ENABLE_DUMP >> > enabled. >> > >>Regards Dibyendu