[PATCH] instcombine: Only create library call simplifier once

Meador Inge meadori at codesourcery.com
Thu Mar 7 12:52:26 PST 2013


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




More information about the llvm-commits mailing list