[PATCH] instcombine: Only create library call simplifier once
Nadav Rotem
nrotem at apple.com
Thu Mar 7 15:30:21 PST 2013
Hi Meador,
Thanks for working on this. I am glad that we are working towards a design that will call initOptimizations once per module, and not once per function. This is critical for compiling C++ code. After we solve this issue we need to think about a way to reduce the compile time of modules that have a single small function. This is common in many domain specific languages, such as OpenCL, or GPU shaders.
The initialization of the LibCallSimplifier looks suspicious. Should we put it in "doInitialization/doFinalization" maybe ?
Thanks,
Nadav
On Mar 7, 2013, at 12:52 PM, Meador Inge <meadori at codesourcery.com> wrote:
> Nadav reported a performance regression due to the work I did to
> merge the library call simplifier into instcombine [1]. The issue
> is that a new LibCallSimplifier object is being created every time
> InstCombiner::runOnFunction is called. The is very inefficient
> because every time a LibCallSimplifier object is new'd up it creates
> a hash table to map from a function name to an object that optimizes
> functions of that name.
>
> The original library call simplifier avoided this problem by only
> initializing that table *once* in SimplifyLibCalls::runOnFunction.
> This patch fixes the problem by implementing a similar idea in
> instcombine. LibCallSimplifier objects are only created the first
> time InstCombiner::runOnFunction is called.
>
> OK?
>
> [1] http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130304/167639.html
> ---
> include/llvm/Transforms/Utils/SimplifyLibCalls.h | 4 ++--
> lib/Transforms/InstCombine/InstCombine.h | 5 ++++-
> lib/Transforms/InstCombine/InstCombineCalls.cpp | 2 +-
> lib/Transforms/InstCombine/InstructionCombining.cpp | 9 ++++-----
> lib/Transforms/Utils/SimplifyLibCalls.cpp | 17 +++++++----------
> 5 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/include/llvm/Transforms/Utils/SimplifyLibCalls.h b/include/llvm/Transforms/Utils/SimplifyLibCalls.h
> index 6bb81be..c636e1a 100644
> --- a/include/llvm/Transforms/Utils/SimplifyLibCalls.h
> +++ b/include/llvm/Transforms/Utils/SimplifyLibCalls.h
> @@ -31,7 +31,7 @@ namespace llvm {
> /// simplifier.
> LibCallSimplifierImpl *Impl;
> public:
> - LibCallSimplifier(const DataLayout *TD, const TargetLibraryInfo *TLI,
> + LibCallSimplifier(const TargetLibraryInfo *TLI,
> bool UnsafeFPShrink);
> virtual ~LibCallSimplifier();
>
> @@ -41,7 +41,7 @@ namespace llvm {
> /// be equal to the instruction being optimized. In this case all
> /// other instructions that use the given instruction were modified
> /// and the given instruction is dead.
> - Value *optimizeCall(CallInst *CI);
> + Value *optimizeCall(CallInst *CI, const DataLayout *TD);
>
> /// replaceAllUsesWith - This method is used when the library call
> /// simplifier needs to replace instructions other than the library
> diff --git a/lib/Transforms/InstCombine/InstCombine.h b/lib/Transforms/InstCombine/InstCombine.h
> index 1f6a3a5e..6ec3786 100644
> --- a/lib/Transforms/InstCombine/InstCombine.h
> +++ b/lib/Transforms/InstCombine/InstCombine.h
> @@ -87,11 +87,14 @@ public:
> BuilderTy *Builder;
>
> static char ID; // Pass identification, replacement for typeid
> - InstCombiner() : FunctionPass(ID), TD(0), Builder(0) {
> + InstCombiner() : FunctionPass(ID), TD(0), Simplifier(0), Builder(0) {
> MinimizeSize = false;
> initializeInstCombinerPass(*PassRegistry::getPassRegistry());
> }
>
> + ~InstCombiner() {
> + delete Simplifier;
> + }
> public:
> virtual bool runOnFunction(Function &F);
>
> diff --git a/lib/Transforms/InstCombine/InstCombineCalls.cpp b/lib/Transforms/InstCombine/InstCombineCalls.cpp
> index 64cd1bd..848a790 100644
> --- a/lib/Transforms/InstCombine/InstCombineCalls.cpp
> +++ b/lib/Transforms/InstCombine/InstCombineCalls.cpp
> @@ -790,7 +790,7 @@ static bool isSafeToEliminateVarargsCast(const CallSite CS,
> Instruction *InstCombiner::tryOptimizeCall(CallInst *CI, const DataLayout *TD) {
> if (CI->getCalledFunction() == 0) return 0;
>
> - if (Value *With = Simplifier->optimizeCall(CI)) {
> + if (Value *With = Simplifier->optimizeCall(CI, TD)) {
> ++NumSimplified;
> return CI->use_empty() ? CI : ReplaceInstUsesWith(*CI, With);
> }
> diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp
> index c6115e3..327c70c 100644
> --- a/lib/Transforms/InstCombine/InstructionCombining.cpp
> +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp
> @@ -2456,10 +2456,9 @@ namespace {
> class InstCombinerLibCallSimplifier : public LibCallSimplifier {
> InstCombiner *IC;
> public:
> - InstCombinerLibCallSimplifier(const DataLayout *TD,
> - const TargetLibraryInfo *TLI,
> + InstCombinerLibCallSimplifier(const TargetLibraryInfo *TLI,
> InstCombiner *IC)
> - : LibCallSimplifier(TD, TLI, UnsafeFPShrink) {
> + : LibCallSimplifier(TLI, UnsafeFPShrink) {
> this->IC = IC;
> }
>
> @@ -2485,8 +2484,8 @@ bool InstCombiner::runOnFunction(Function &F) {
> InstCombineIRInserter(Worklist));
> Builder = &TheBuilder;
>
> - InstCombinerLibCallSimplifier TheSimplifier(TD, TLI, this);
> - Simplifier = &TheSimplifier;
> + if (!Simplifier)
> + Simplifier = new InstCombinerLibCallSimplifier(TLI, this);
>
> bool EverMadeChange = false;
>
> diff --git a/lib/Transforms/Utils/SimplifyLibCalls.cpp b/lib/Transforms/Utils/SimplifyLibCalls.cpp
> index 8ff87ee..29cece4 100644
> --- a/lib/Transforms/Utils/SimplifyLibCalls.cpp
> +++ b/lib/Transforms/Utils/SimplifyLibCalls.cpp
> @@ -1668,7 +1668,6 @@ struct PutsOpt : public LibCallOptimization {
> namespace llvm {
>
> class LibCallSimplifierImpl {
> - const DataLayout *TD;
> const TargetLibraryInfo *TLI;
> const LibCallSimplifier *LCS;
> bool UnsafeFPShrink;
> @@ -1728,18 +1727,17 @@ class LibCallSimplifierImpl {
> void addOpt(LibFunc::Func F, LibCallOptimization* Opt);
> void addOpt(LibFunc::Func F1, LibFunc::Func F2, LibCallOptimization* Opt);
> public:
> - LibCallSimplifierImpl(const DataLayout *TD, const TargetLibraryInfo *TLI,
> + LibCallSimplifierImpl(const TargetLibraryInfo *TLI,
> const LibCallSimplifier *LCS,
> bool UnsafeFPShrink = false)
> : UnaryDoubleFP(false), UnsafeUnaryDoubleFP(true),
> Cos(UnsafeFPShrink), Pow(UnsafeFPShrink), Exp2(UnsafeFPShrink) {
> - this->TD = TD;
> this->TLI = TLI;
> this->LCS = LCS;
> this->UnsafeFPShrink = UnsafeFPShrink;
> }
>
> - Value *optimizeCall(CallInst *CI);
> + Value *optimizeCall(CallInst *CI, const DataLayout *TD);
> };
>
> void LibCallSimplifierImpl::initOptimizations() {
> @@ -1854,7 +1852,7 @@ void LibCallSimplifierImpl::initOptimizations() {
> addOpt(LibFunc::puts, &Puts);
> }
>
> -Value *LibCallSimplifierImpl::optimizeCall(CallInst *CI) {
> +Value *LibCallSimplifierImpl::optimizeCall(CallInst *CI, const DataLayout *TD) {
> if (Optimizations.empty())
> initOptimizations();
>
> @@ -1878,19 +1876,18 @@ void LibCallSimplifierImpl::addOpt(LibFunc::Func F1, LibFunc::Func F2,
> Optimizations[TLI->getName(F1)] = Opt;
> }
>
> -LibCallSimplifier::LibCallSimplifier(const DataLayout *TD,
> - const TargetLibraryInfo *TLI,
> +LibCallSimplifier::LibCallSimplifier(const TargetLibraryInfo *TLI,
> bool UnsafeFPShrink) {
> - Impl = new LibCallSimplifierImpl(TD, TLI, this, UnsafeFPShrink);
> + Impl = new LibCallSimplifierImpl(TLI, this, UnsafeFPShrink);
> }
>
> LibCallSimplifier::~LibCallSimplifier() {
> delete Impl;
> }
>
> -Value *LibCallSimplifier::optimizeCall(CallInst *CI) {
> +Value *LibCallSimplifier::optimizeCall(CallInst *CI, const DataLayout *TD) {
> if (CI->hasFnAttr(Attribute::NoBuiltin)) return 0;
> - return Impl->optimizeCall(CI);
> + return Impl->optimizeCall(CI, TD);
> }
>
> void LibCallSimplifier::replaceAllUsesWith(Instruction *I, Value *With) const {
> --
> 1.8.1
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130307/10cf0d45/attachment.html>
More information about the llvm-commits
mailing list