[feature suggestion] Please split out dump() and other debug functionality into a separate library

[Quuxplusone]

Bugzilla Link	PR40875
Status	NEW
Importance	P enhancement
Reported by	Yuri (yuri@tsoft.com)
Reported on	2019-02-26 11:40:59 -0800
Last modified on	2019-02-27 11:58:13 -0800
Version	trunk
Hardware	PC FreeBSD
CC	babokin@gmail.com, dblaikie@gmail.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

The Intel's ISPC compiler [1] and flang compiler expect dump() function to be
present while it is commonly excluded in the packages. LLVM_ENABLE_DUMP is a
relevant cmake variable.

It is difficult to build LLVM with custom options for individual other projects.

Suggested solution:
Please add a separate library that would contain functions disabled by
LLVM_ENABLE_DUMP=OFF, for example libLLVMDebug.so

This library can be installed as a sub-package, or as a separate package, and
would eliminate the problem.

---References---
[1] ISPS bug report. https://github.com/ispc/ispc/issues/1427

[Quuxplusone]

I'm not sure that it's good way forward. This would require moving all dump() functions to a separate compilation module. Having dump() functions as member functions of various classes is really nice. And keeping them member functions and moving them to a separate compilation unit at the same time seems to be an overkill.

[Quuxplusone]

Having dump() functions as member functions of various classes is really nice.

I didn't mean to move them away from their classes. They would remain members of the same classes, but their implementations would move into a separate library.

[Quuxplusone]

(In reply to Yuri from comment #2)
> > Having dump() functions as member functions of various classes is really
nice.
>
> I didn't mean to move them away from their classes. They would remain
> members of the same classes, but their implementations would move into a
> separate library.

Splitting declaration/definition location across libraries like that can make
the code a bit hard to maintain (& the bit I care about: it can break Clang
modules builds that use modular codegen - though this is an experimental mode,
it's also a pretty good way to keep good layering in libraries that would
specifically prevent this sort of layout)

[Quuxplusone]

To come back to the original question - how is it that these external projects rely on LLVM's dump functions? Seems like maybe those projects shoudl be modified to not do that.

Those functions are meant for developers in debuggers - not for any actual product functionality. (if those projects in turn have their own dump functions that call LLVM's - probably gate them on the same macro, etc)

[Quuxplusone]

(In reply to David Blaikie from comment #4)
> To come back to the original question - how is it that these external
> projects rely on LLVM's dump functions? Seems like maybe those projects
> shoudl be modified to not do that.
>
> Those functions are meant for developers in debuggers - not for any actual
> product functionality. (if those projects in turn have their own dump
> functions that call LLVM's - probably gate them on the same macro, etc)

Intel's ISPC devs said that they consider 'dump()' core LLVM functionality.
This approach leads to packaging difficulties. My suggestion here tries to
mitigate the problem by allowing the debug functionality to be packageable
separately, as a sub-package or a separate package.

[Quuxplusone]

(In reply to Yuri from comment #5)
> (In reply to David Blaikie from comment #4)
> > To come back to the original question - how is it that these external
> > projects rely on LLVM's dump functions? Seems like maybe those projects
> > shoudl be modified to not do that.
> >
> > Those functions are meant for developers in debuggers - not for any actual
> > product functionality. (if those projects in turn have their own dump
> > functions that call LLVM's - probably gate them on the same macro, etc)
>
>
> Intel's ISPC devs said that they consider 'dump()' core LLVM functionality.
> This approach leads to packaging difficulties. My suggestion here tries to
> mitigate the problem by allowing the debug functionality to be packageable
> separately, as a sub-package or a separate package.

It seems like they're mistaken though, right - given the reality of what's
packaged/shipped in release builds of LLVM (not just distro builds, but the
official ones that go on the LLVM website)?

[Quuxplusone]

(In reply to David Blaikie from comment #6)
o be packageable
> > separately, as a sub-package or a separate package.
>
> It seems like they're mistaken though, right - given the reality of what's
> packaged/shipped in release builds of LLVM (not just distro builds, but the
> official ones that go on the LLVM website)?

They are suggesting users to build custom version of LLVM only for the ISPC use.
On BSDs LLVM requires some patching to build, with patches stored in the llvm
port, so it isn't easy to build custom versions for individual consumers.

[Quuxplusone]

ISPC is NOT supposed to be built with official LLVM package. It may be built with official or distro-specific LLVM package, but in this case, it will have a number of stability and performance issues. So we require custom LLVM build for ISPC. So I don't think the issue with dump() is critical in any way.

For the question why we need custom build - ISPC exposes number of issues in LLVM that are mostly specific to the way ISPC uses LLVM. We file bugs against LLVM, they got fixed and we port the fixes back to specific LLVM version that is used for our release build. Maintaining ISPC in sync with LLVM at any given point in time requires a lot of efforts, so we are a bit behind LLVM - typically we do clean up and stabilization after the LLVM is released, so fixes for our bugs need to be back-ported and are not included in already released versions.

[Quuxplusone]

Do you plan to include BSD-specific patches LLVM upstream? This seems to be proper way go forward.

[Quuxplusone]

(In reply to Dmitry Babokin from comment #9)
> Do you plan to include BSD-specific patches LLVM upstream? This seems to be
> proper way go forward.

I don't maintain llvm ports, but yes, some parts can be upstreamed.

Same goes for ISPC. When you say that some "stability patches" are required,
this implies that LLVM isn't itself stable without these patches. Some other
users can also benefit from these patches, so they should be upstreamed.

[Quuxplusone]

The patches that we require for LLVM build are already in upstream, we are back-porting them to older versions. The problem is that we discover these bugs and fix them after the LLVM that we are using for build is released, so release version doesn't contain these patches.

Also, in most of the cases the bugs that we expose in LLVM are not affecting majority of clang users, as quite frequently we trigger significantly different code generation than typical clang program.