This is an archive of the discontinued LLVM Phabricator instance.

[cmake] Add support for explicit STATIC libraries even when building mostly shared libraries.
AbandonedPublic

Authored by stephenneuendorffer on Jan 12 2020, 11:39 PM.

Details

Summary

By default when BUILD_SHARED_LIBS=ON, all libraries are forced to be
shared. However, with circular dependencies between libraries this
becomes a problem. This change allows libraries to be explicitly
marked as static in CMAKE to solve circular dependencies while getting
most of the build-time advantages from BUILD_SHARED_LIBS=ON

[examples] Fix CMakefiles for JITLink and OrcError library refactoring

The examples need explicit library dependencies when building with
BUILD_SHARED_LIBS=on

[MLIR] Mark application libraries as "BUILDTREE_ONLY"

These libraries are intended to be used as part of specific
applications (like mlir-opt). If they are both included in a single
library (e.g. libLLVM.so), then their options will collide, because
static initializers will be called more than once.

[MLIR] Fix OpDefinition::classof check for shared libraries.

When using MLIR in a shared library, it is possible for the shared library
to load a different, but equivalent version of a class method. As a result,
function pointer comparison is not a reliable check of class equivalence.
This causes problems related to this code in OpDefinition.h:

/// Return true if this "op class" can match against the specified operation.
static bool classof(Operation *op) {
  if (auto *abstractOp = op->getAbstractOperation()) {
    return &classof == abstractOp->classof;
  }
  return op->getName().getStringRef() == ConcreteType::getOperationName();
}

It appears that the intention is to implement the normal LLVM cast<>/isa<>
mechanism, but without a fixed enum of Kinds. Primarily, the operation
name check allows casting based on the operation name, which are assumed
to be unique for an operation class. (Aside: is this enforced anywhere?)
Before this, another, more efficient, check based on pointer equality of
the classof method. Unfortunately, this pointer equality check fails
if/when the classof method is inlined into a shared object (e.g. libLLVM.so)
and also exists in the main executable, resulting in triggering the assert()
inside cast<>.

This patch fixes this method to fall back to the slow, but accurate string
comparison if the function pointer comparison fails.

[MLIR] Fixes for BUILD_SHARED_LIBS=on

This patch enables the above cmake option, which builds most libraries
as DLLs instead of statically linked libraries. The main effect of this is
that incrememntal build times are greatly reduced, since usually only one
library need be relinked in response to isolated code changes.

The bulk of this patch is fixing incorrect usage of cmake, where library
dependencies are listed under add_dependencies rather than under
target_link_libraries or under the LINK_LIBS tag. Correct usage should be
like this:

add_dependencies(MLIRfoo MLIRfooIncGen)
target_link_libraries(MLIRfoo MLIRlib1 MLIRlib2)

[MLIR] Move from add_llvm_library to add_llvm_component_library

This results in most MLIR libraries being linked into libLLVM.so

Event Timeline

Herald added a project: Restricted Project. · View Herald Transcript

Updating D72583: [cmake] Add support for explicit STATIC libraries even when building mostly shared libraries.

nicolasvasilache accepted this revision.Jan 16 2020, 5:46 AM

Great, thanks!

This revision is now accepted and ready to land.Jan 16 2020, 5:46 AM
mehdi_amini added inline comments.Jan 16 2020, 8:58 PM
mlir/lib/Support/CMakeLists.txt
40

I don't understand this, and this revision description seems to include many other changes (like from other commits), but I expect some doc in the code to explain all this.