[llvm-dev] Errors linking with LLVM 5.0 - dump() missing

[Mehdi AMINI via llvm-dev]

2017-09-26 11:54 GMT-07:00 Matthias Braun via llvm-dev <
llvm-dev at lists.llvm.org>:

>
> > On Sep 26, 2017, at 7:04 AM, David Keaton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >
> > On 09/25/2017 06:47 PM, David Keaton via llvm-dev wrote:
> >> On 09/25/2017 06:19 PM, Matthias Braun wrote:
> >>>
> >>>> On Sep 25, 2017, at 6:03 PM, David Keaton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>>>
> >>>> On 09/25/2017 02:53 PM, Matthias Braun via llvm-dev wrote:
> >>> In the meantime `cmake -DCMAKE_CXX_FLAGS="-DLLVM_ENABLE_DUMP"` should
> do the trick.
> >>      Thank you.  That was the flag we needed.
> >>      Unfortunately, when I tried this just now, I found that the
> declaration of llvm::MachineRegisterInfo::dumpUses() in
> include/llvm/CodeGen/MachineRegisterInfo.h is missing the check against
> LLVM_ENABLE_DUMP.  It has only
> >> #ifndef NDEBUG
> >> which causes the build to fail because the definition in lib/CodeGen/MachineRegisterInfo.cpp
> is guarded by this.
> >> #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
> >
> >     Unfortunately, this was only the beginning.  I tried updating the
> header file to test LLVM_ENABLE_DUMP as well, and when I did that, the
> build hit another such case, which I tried fixing, and then it hit another,
> and another.  Here is what I have found so far that would need to be fixed
> under the current arrangement.
> >
> > LLVM_ENABLE_DUMP added to:
> >
> > include/llvm/CodeGen/MachineRegisterInfo.h
> >       dumpUses()
> >
> > include/llvm/CodeGen/MachineScheduler.h
> >       dumpScheduledState()
> >
> > include/llvm/CodeGen/TargetSchedule.h
> >       getResourceName()
> >
> > include/llvm/MC/MCSchedule.h
> >       Name
> >
> > utils/TableGen/SubtargetEmitter.cpp
> >       emitted code inside EmitSchedModel()
> >
> > Unknown location in AArch64
> >       This is not the end.  I just lost track of what to change at this
> point.
> I guess that was to be expected with an option that didn't even have a
> cmake flag. I would propose to just check for LLVM_ENABLE_DUMP in the
> sourcecode and move the logic that debug builds have dump() methods to the
> cmake side of things to simplify things. But I think that needs agreement
> and someone to write a patch.
>
> +CC some of the people who I believe made the decision for the current
> style.
>

The usual proper way to expose such options (IIRC) is to define such config
options in the llvm-config.h (or config.h I forgot which one) during the
CMake configuration, and clients are supposed to use the definition from
these header that comes with the pre-built LLVM to conditionally compile
out their calls to these.



>
> >
> >     Perhaps it would be easier to revert the change that hid the dump()
> methods.  That would have another advantage as well.  Before LLVM 5.0.0,
> Chapel had the ability to link with an existing pre-built LLVM if it was
> installed on the system. With LLVM 5.0.0, we cannot do that any longer
> because we need to compile with LLVM_ENABLE_DUMP defined.  It takes 10x
> longer to compile LLVM than it does to compile the rest of Chapel, so it is
> quite a burden to require a recompilation of LLVM to get the dump() methods.

Again: I consider client code calling dump() to be a misuse.
>

I tend to agree, but there is a principled / robust way to have a client
support debug features alongside with a debug build of LLVM (the config.h
option I mentioned above).

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170926/3ec03a92/attachment.html>