Skip to content

Commit

Permalink
[PM] Rework how the TargetLibraryInfo pass integrates with the new pass
Browse files Browse the repository at this point in the history
manager to support the actual uses of it. =]

When I ported instcombine to the new pass manager I discover that it
didn't work because TLI wasn't available in the right places. This is
a somewhat surprising and/or subtle aspect of the new pass manager
design that came up before but I think is useful to be reminded of:

While the new pass manager *allows* a function pass to query a module
analysis, it requires that the module analysis is already run and cached
prior to the function pass manager starting up, possibly with
a 'require<foo>' style utility in the pass pipeline. This is an
intentional hurdle because using a module analysis from a function pass
*requires* that the module analysis is run prior to entering the
function pass manager. Otherwise the other functions in the module could
be in who-knows-what state, etc.

A somewhat surprising consequence of this design decision (at least to
me) is that you have to design a function pass that leverages
a module analysis to do so as an optional feature. Even if that means
your function pass does no work in the absence of the module analysis,
you have to handle that possibility and remain conservatively correct.
This is a natural consequence of things being able to invalidate the
module analysis and us being unable to re-run it. And it's a generally
good thing because it lets us reorder passes arbitrarily without
breaking correctness, etc.

This ends up causing problems in one case. What if we have a module
analysis that is *definitionally* impossible to invalidate. In the
places this might come up, the analysis is usually also definitionally
trivial to run even while other transformation passes run on the module,
regardless of the state of anything. And so, it follows that it is
natural to have a hard requirement on such analyses from a function
pass.

It turns out, that TargetLibraryInfo is just such an analysis, and
InstCombine has a hard requirement on it.

The approach I've taken here is to produce an analysis that models this
flexibility by making it both a module and a function analysis. This
exposes the fact that it is in fact safe to compute at any point. We can
even make it a valid CGSCC analysis at some point if that is useful.
However, we don't want to have a copy of the actual target library info
state for each function! This state is specific to the triple. The
somewhat direct and blunt approach here is to turn TLI into a pimpl,
with the state and mutators in the implementation class and the query
routines primarily in the wrapper. Then the analysis can lazily
construct and cache the implementations, keyed on the triple, and
on-demand produce wrappers of them for each function.

One minor annoyance is that we will end up with a wrapper for each
function in the module. While this is a bit wasteful (one pointer per
function) it seems tolerable. And it has the advantage of ensuring that
we pay the absolute minimum synchronization cost to access this
information should we end up with a nice parallel function pass manager
in the future. We could look into trying to mark when analysis results
are especially cheap to recompute and more eagerly GC-ing the cached
results, or we could look at supporting a variant of analyses whose
results are specifically *not* cached and expected to just be used and
discarded by the consumer. Either way, these seem like incremental
enhancements that should happen when we start profiling the memory and
CPU usage of the new pass manager and not before.

The other minor annoyance is that if we end up using the TLI in both
a module pass and a function pass, those will be produced by two
separate analyses, and thus will point to separate copies of the
implementation state. While a minor issue, I dislike this and would like
to find a way to cleanly allow a single analysis instance to be used
across multiple IR unit managers. But I don't have a good solution to
this today, and I don't want to hold up all of the work waiting to come
up with one. This too seems like a reasonable thing to incrementally
improve later.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226981 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chandlerc committed Jan 24, 2015
1 parent 104a21d commit 6f409cb
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 95 deletions.
3 changes: 3 additions & 0 deletions include/llvm/ADT/Triple.h
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ class Triple {
/// common case in which otherwise valid components are in the wrong order.
static std::string normalize(StringRef Str);

/// \brief Return the normalized form of this triple's string.
std::string normalize() const { return normalize(Data); }

/// @}
/// @name Typed Component Access
/// @{
Expand Down
166 changes: 100 additions & 66 deletions include/llvm/Analysis/TargetLibraryInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/Triple.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/Module.h"
#include "llvm/Pass.h"

Expand Down Expand Up @@ -696,12 +696,15 @@ class PreservedAnalyses;
};
}

/// \brief Provides information about what library functions are available for
/// the current target.
/// \brief Implementation of the target library information.
///
/// This both allows optimizations to handle them specially and frontends to
/// disable such optimizations through -fno-builtin etc.
class TargetLibraryInfo {
/// This class constructs tables that hold the target library information and
/// make it available. However, it is somewhat expensive to compute and only
/// depends on the triple. So users typicaly interact with the \c
/// TargetLibraryInfo wrapper below.
class TargetLibraryInfoImpl {
friend class TargetLibraryInfo;

unsigned char AvailableArray[(LibFunc::NumLibFuncs+3)/4];
llvm::DenseMap<unsigned, std::string> CustomNames;
static const char* StandardNames[LibFunc::NumLibFuncs];
Expand All @@ -720,30 +723,92 @@ class TargetLibraryInfo {
}

public:
TargetLibraryInfo();
explicit TargetLibraryInfo(const Triple &T);
TargetLibraryInfoImpl();
explicit TargetLibraryInfoImpl(const Triple &T);

// Provide value semantics.
TargetLibraryInfo(const TargetLibraryInfo &TLI);
TargetLibraryInfo(TargetLibraryInfo &&TLI);
TargetLibraryInfo &operator=(const TargetLibraryInfo &TLI);
TargetLibraryInfo &operator=(TargetLibraryInfo &&TLI);
TargetLibraryInfoImpl(const TargetLibraryInfoImpl &TLI);
TargetLibraryInfoImpl(TargetLibraryInfoImpl &&TLI);
TargetLibraryInfoImpl &operator=(const TargetLibraryInfoImpl &TLI);
TargetLibraryInfoImpl &operator=(TargetLibraryInfoImpl &&TLI);

/// \brief Searches for a particular function name.
///
/// If it is one of the known library functions, return true and set F to the
/// corresponding value.
bool getLibFunc(StringRef funcName, LibFunc::Func &F) const;

/// \brief Forces a function to be marked as unavailable.
void setUnavailable(LibFunc::Func F) {
setState(F, Unavailable);
}

/// \brief Forces a function to be marked as available.
void setAvailable(LibFunc::Func F) {
setState(F, StandardName);
}

/// \brief Forces a function to be marked as available and provide an
/// alternate name that must be used.
void setAvailableWithName(LibFunc::Func F, StringRef Name) {
if (StandardNames[F] != Name) {
setState(F, CustomName);
CustomNames[F] = Name;
assert(CustomNames.find(F) != CustomNames.end());
} else {
setState(F, StandardName);
}
}

/// \brief Disables all builtins.
///
/// This can be used for options like -fno-builtin.
void disableAllFunctions();
};

/// \brief Provides information about what library functions are available for
/// the current target.
///
/// This both allows optimizations to handle them specially and frontends to
/// disable such optimizations through -fno-builtin etc.
class TargetLibraryInfo {
friend class TargetLibraryAnalysis;
friend class TargetLibraryInfoWrapperPass;

const TargetLibraryInfoImpl *Impl;

public:
explicit TargetLibraryInfo(const TargetLibraryInfoImpl &Impl) : Impl(&Impl) {}

// Provide value semantics.
TargetLibraryInfo(const TargetLibraryInfo &TLI) : Impl(TLI.Impl) {}
TargetLibraryInfo(TargetLibraryInfo &&TLI) : Impl(TLI.Impl) {}
TargetLibraryInfo &operator=(const TargetLibraryInfo &TLI) {
Impl = TLI.Impl;
return *this;
}
TargetLibraryInfo &operator=(TargetLibraryInfo &&TLI) {
Impl = TLI.Impl;
return *this;
}

/// \brief Searches for a particular function name.
///
/// If it is one of the known library functions, return true and set F to the
/// corresponding value.
bool getLibFunc(StringRef funcName, LibFunc::Func &F) const {
return Impl->getLibFunc(funcName, F);
}

/// \brief Tests wether a library function is available.
bool has(LibFunc::Func F) const {
return getState(F) != Unavailable;
return Impl->getState(F) != TargetLibraryInfoImpl::Unavailable;
}

/// \brief Tests if the function is both available and a candidate for
/// optimized code generation.
bool hasOptimizedCodeGen(LibFunc::Func F) const {
if (getState(F) == Unavailable)
if (Impl->getState(F) == TargetLibraryInfoImpl::Unavailable)
return false;
switch (F) {
default: break;
Expand Down Expand Up @@ -773,42 +838,15 @@ class TargetLibraryInfo {
}

StringRef getName(LibFunc::Func F) const {
AvailabilityState State = getState(F);
if (State == Unavailable)
auto State = Impl->getState(F);
if (State == TargetLibraryInfoImpl::Unavailable)
return StringRef();
if (State == StandardName)
return StandardNames[F];
assert(State == CustomName);
return CustomNames.find(F)->second;
if (State == TargetLibraryInfoImpl::StandardName)
return Impl->StandardNames[F];
assert(State == TargetLibraryInfoImpl::CustomName);
return Impl->CustomNames.find(F)->second;
}

/// \brief Forces a function to be marked as unavailable.
void setUnavailable(LibFunc::Func F) {
setState(F, Unavailable);
}

/// \brief Forces a function to be marked as available.
void setAvailable(LibFunc::Func F) {
setState(F, StandardName);
}

/// \brief Forces a function to be marked as available and provide an
/// alternate name that must be used.
void setAvailableWithName(LibFunc::Func F, StringRef Name) {
if (StandardNames[F] != Name) {
setState(F, CustomName);
CustomNames[F] = Name;
assert(CustomNames.find(F) != CustomNames.end());
} else {
setState(F, StandardName);
}
}

/// \brief Disables all builtins.
///
/// This can be used for options like -fno-builtin.
void disableAllFunctions();

/// \brief Handle invalidation from the pass manager.
///
/// If we try to invalidate this info, just return false. It cannot become
Expand Down Expand Up @@ -837,40 +875,36 @@ class TargetLibraryAnalysis {
///
/// This will directly copy the preset info into the result without
/// consulting the module's triple.
TargetLibraryAnalysis(TargetLibraryInfo PresetInfo)
: PresetInfo(std::move(PresetInfo)) {}
TargetLibraryAnalysis(TargetLibraryInfoImpl PresetInfoImpl)
: PresetInfoImpl(std::move(PresetInfoImpl)) {}

// Value semantics. We spell out the constructors for MSVC.
TargetLibraryAnalysis(const TargetLibraryAnalysis &Arg)
: PresetInfo(Arg.PresetInfo) {}
// Move semantics. We spell out the constructors for MSVC.
TargetLibraryAnalysis(TargetLibraryAnalysis &&Arg)
: PresetInfo(std::move(Arg.PresetInfo)) {}
TargetLibraryAnalysis &operator=(const TargetLibraryAnalysis &RHS) {
PresetInfo = RHS.PresetInfo;
return *this;
}
: PresetInfoImpl(std::move(Arg.PresetInfoImpl)), Impls(std::move(Arg.Impls)) {}
TargetLibraryAnalysis &operator=(TargetLibraryAnalysis &&RHS) {
PresetInfo = std::move(RHS.PresetInfo);
PresetInfoImpl = std::move(RHS.PresetInfoImpl);
Impls = std::move(RHS.Impls);
return *this;
}

TargetLibraryInfo run(Module &M) {
if (PresetInfo)
return *PresetInfo;

return TargetLibraryInfo(Triple(M.getTargetTriple()));
}
TargetLibraryInfo run(Module &M);
TargetLibraryInfo run(Function &F);

/// \brief Provide access to a name for this pass for debugging purposes.
static StringRef name() { return "TargetLibraryAnalysis"; }

private:
static char PassID;

Optional<TargetLibraryInfo> PresetInfo;
Optional<TargetLibraryInfoImpl> PresetInfoImpl;

StringMap<std::unique_ptr<TargetLibraryInfoImpl>> Impls;

TargetLibraryInfoImpl &lookupInfoImpl(Triple T);
};

class TargetLibraryInfoWrapperPass : public ImmutablePass {
TargetLibraryInfoImpl TLIImpl;
TargetLibraryInfo TLI;

virtual void anchor();
Expand All @@ -879,7 +913,7 @@ class TargetLibraryInfoWrapperPass : public ImmutablePass {
static char ID;
TargetLibraryInfoWrapperPass();
explicit TargetLibraryInfoWrapperPass(const Triple &T);
explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfo &TLI);
explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfoImpl &TLI);

TargetLibraryInfo &getTLI() { return TLI; }
const TargetLibraryInfo &getTLI() const { return TLI; }
Expand Down
4 changes: 2 additions & 2 deletions include/llvm/Transforms/IPO/PassManagerBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

namespace llvm {
class Pass;
class TargetLibraryInfo;
class TargetLibraryInfoImpl;
class TargetMachine;

// The old pass manager infrastructure is hidden in a legacy namespace now.
Expand Down Expand Up @@ -105,7 +105,7 @@ class PassManagerBuilder {
/// LibraryInfo - Specifies information about the runtime library for the
/// optimizer. If this is non-null, it is added to both the function and
/// per-module pass pipeline.
TargetLibraryInfo *LibraryInfo;
TargetLibraryInfoImpl *LibraryInfo;

/// Inliner - Specifies the inliner to use. If this is non-null, it is
/// added to the per-module passes.
Expand Down
Loading

0 comments on commit 6f409cb

Please sign in to comment.