<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi Meador, </div><div><br></div><div>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.</div><div><br></div><div>The initialization of the LibCallSimplifier looks suspicious. Should we put it in "doInitialization/doFinalization" maybe ?</div><div><br></div><div>Thanks,</div><div>Nadav</div><div><br></div><div><br></div><br><div><div>On Mar 7, 2013, at 12:52 PM, Meador Inge <<a href="mailto:meadori@codesourcery.com">meadori@codesourcery.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">Nadav reported a performance regression due to the work I did to<br>merge the library call simplifier into instcombine [1]. The issue<br>is that a new LibCallSimplifier object is being created every time<br>InstCombiner::runOnFunction is called. The is very inefficient<br>because every time a LibCallSimplifier object is new'd up it creates<br>a hash table to map from a function name to an object that optimizes<br>functions of that name.<br><br>The original library call simplifier avoided this problem by only<br>initializing that table *once* in SimplifyLibCalls::runOnFunction.<br>This patch fixes the problem by implementing a similar idea in<br>instcombine. LibCallSimplifier objects are only created the first<br>time InstCombiner::runOnFunction is called.<br><br>OK?<br><br>[1]<span class="Apple-converted-space"> </span><a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130304/167639.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130304/167639.html</a><br>---<br>include/llvm/Transforms/Utils/SimplifyLibCalls.h | 4 ++--<br>lib/Transforms/InstCombine/InstCombine.h | 5 ++++-<br>lib/Transforms/InstCombine/InstCombineCalls.cpp | 2 +-<br>lib/Transforms/InstCombine/InstructionCombining.cpp | 9 ++++-----<br>lib/Transforms/Utils/SimplifyLibCalls.cpp | 17 +++++++----------<br>5 files changed, 18 insertions(+), 19 deletions(-)<br><br>diff --git a/include/llvm/Transforms/Utils/SimplifyLibCalls.h b/include/llvm/Transforms/Utils/SimplifyLibCalls.h<br>index 6bb81be..c636e1a 100644<br>--- a/include/llvm/Transforms/Utils/SimplifyLibCalls.h<br>+++ b/include/llvm/Transforms/Utils/SimplifyLibCalls.h<br>@@ -31,7 +31,7 @@ namespace llvm {<br> /// simplifier.<br> LibCallSimplifierImpl *Impl;<br> public:<br>- LibCallSimplifier(const DataLayout *TD, const TargetLibraryInfo *TLI,<br>+ LibCallSimplifier(const TargetLibraryInfo *TLI,<br> bool UnsafeFPShrink);<br> virtual ~LibCallSimplifier();<br><br>@@ -41,7 +41,7 @@ namespace llvm {<br> /// be equal to the instruction being optimized. In this case all<br> /// other instructions that use the given instruction were modified<br> /// and the given instruction is dead.<br>- Value *optimizeCall(CallInst *CI);<br>+ Value *optimizeCall(CallInst *CI, const DataLayout *TD);<br><br> /// replaceAllUsesWith - This method is used when the library call<br> /// simplifier needs to replace instructions other than the library<br>diff --git a/lib/Transforms/InstCombine/InstCombine.h b/lib/Transforms/InstCombine/InstCombine.h<br>index 1f6a3a5e..6ec3786 100644<br>--- a/lib/Transforms/InstCombine/InstCombine.h<br>+++ b/lib/Transforms/InstCombine/InstCombine.h<br>@@ -87,11 +87,14 @@ public:<br> BuilderTy *Builder;<br><br> static char ID; // Pass identification, replacement for typeid<br>- InstCombiner() : FunctionPass(ID), TD(0), Builder(0) {<br>+ InstCombiner() : FunctionPass(ID), TD(0), Simplifier(0), Builder(0) {<br> MinimizeSize = false;<br> initializeInstCombinerPass(*PassRegistry::getPassRegistry());<br> }<br><br>+ ~InstCombiner() {<br>+ delete Simplifier;<br>+ }<br>public:<br> virtual bool runOnFunction(Function &F);<br><br>diff --git a/lib/Transforms/InstCombine/InstCombineCalls.cpp b/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>index 64cd1bd..848a790 100644<br>--- a/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>+++ b/lib/Transforms/InstCombine/InstCombineCalls.cpp<br>@@ -790,7 +790,7 @@ static bool isSafeToEliminateVarargsCast(const CallSite CS,<br>Instruction *InstCombiner::tryOptimizeCall(CallInst *CI, const DataLayout *TD) {<br> if (CI->getCalledFunction() == 0) return 0;<br><br>- if (Value *With = Simplifier->optimizeCall(CI)) {<br>+ if (Value *With = Simplifier->optimizeCall(CI, TD)) {<br> ++NumSimplified;<br> return CI->use_empty() ? CI : ReplaceInstUsesWith(*CI, With);<br> }<br>diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp<br>index c6115e3..327c70c 100644<br>--- a/lib/Transforms/InstCombine/InstructionCombining.cpp<br>+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp<br>@@ -2456,10 +2456,9 @@ namespace {<br>class InstCombinerLibCallSimplifier : public LibCallSimplifier {<br> InstCombiner *IC;<br>public:<br>- InstCombinerLibCallSimplifier(const DataLayout *TD,<br>- const TargetLibraryInfo *TLI,<br>+ InstCombinerLibCallSimplifier(const TargetLibraryInfo *TLI,<br> InstCombiner *IC)<br>- : LibCallSimplifier(TD, TLI, UnsafeFPShrink) {<br>+ : LibCallSimplifier(TLI, UnsafeFPShrink) {<br> this->IC = IC;<br> }<br><br>@@ -2485,8 +2484,8 @@ bool InstCombiner::runOnFunction(Function &F) {<br> InstCombineIRInserter(Worklist));<br> Builder = &TheBuilder;<br><br>- InstCombinerLibCallSimplifier TheSimplifier(TD, TLI, this);<br>- Simplifier = &TheSimplifier;<br>+ if (!Simplifier)<br>+ Simplifier = new InstCombinerLibCallSimplifier(TLI, this);<br><br> bool EverMadeChange = false;<br><br>diff --git a/lib/Transforms/Utils/SimplifyLibCalls.cpp b/lib/Transforms/Utils/SimplifyLibCalls.cpp<br>index 8ff87ee..29cece4 100644<br>--- a/lib/Transforms/Utils/SimplifyLibCalls.cpp<br>+++ b/lib/Transforms/Utils/SimplifyLibCalls.cpp<br>@@ -1668,7 +1668,6 @@ struct PutsOpt : public LibCallOptimization {<br>namespace llvm {<br><br>class LibCallSimplifierImpl {<br>- const DataLayout *TD;<br> const TargetLibraryInfo *TLI;<br> const LibCallSimplifier *LCS;<br> bool UnsafeFPShrink;<br>@@ -1728,18 +1727,17 @@ class LibCallSimplifierImpl {<br> void addOpt(LibFunc::Func F, LibCallOptimization* Opt);<br> void addOpt(LibFunc::Func F1, LibFunc::Func F2, LibCallOptimization* Opt);<br>public:<br>- LibCallSimplifierImpl(const DataLayout *TD, const TargetLibraryInfo *TLI,<br>+ LibCallSimplifierImpl(const TargetLibraryInfo *TLI,<br> const LibCallSimplifier *LCS,<br> bool UnsafeFPShrink = false)<br> : UnaryDoubleFP(false), UnsafeUnaryDoubleFP(true),<br> Cos(UnsafeFPShrink), Pow(UnsafeFPShrink), Exp2(UnsafeFPShrink) {<br>- this->TD = TD;<br> this->TLI = TLI;<br> this->LCS = LCS;<br> this->UnsafeFPShrink = UnsafeFPShrink;<br> }<br><br>- Value *optimizeCall(CallInst *CI);<br>+ Value *optimizeCall(CallInst *CI, const DataLayout *TD);<br>};<br><br>void LibCallSimplifierImpl::initOptimizations() {<br>@@ -1854,7 +1852,7 @@ void LibCallSimplifierImpl::initOptimizations() {<br> addOpt(LibFunc::puts, &Puts);<br>}<br><br>-Value *LibCallSimplifierImpl::optimizeCall(CallInst *CI) {<br>+Value *LibCallSimplifierImpl::optimizeCall(CallInst *CI, const DataLayout *TD) {<br> if (Optimizations.empty())<br> initOptimizations();<br><br>@@ -1878,19 +1876,18 @@ void LibCallSimplifierImpl::addOpt(LibFunc::Func F1, LibFunc::Func F2,<br> Optimizations[TLI->getName(F1)] = Opt;<br>}<br><br>-LibCallSimplifier::LibCallSimplifier(const DataLayout *TD,<br>- const TargetLibraryInfo *TLI,<br>+LibCallSimplifier::LibCallSimplifier(const TargetLibraryInfo *TLI,<br> bool UnsafeFPShrink) {<br>- Impl = new LibCallSimplifierImpl(TD, TLI, this, UnsafeFPShrink);<br>+ Impl = new LibCallSimplifierImpl(TLI, this, UnsafeFPShrink);<br>}<br><br>LibCallSimplifier::~LibCallSimplifier() {<br> delete Impl;<br>}<br><br>-Value *LibCallSimplifier::optimizeCall(CallInst *CI) {<br>+Value *LibCallSimplifier::optimizeCall(CallInst *CI, const DataLayout *TD) {<br> if (CI->hasFnAttr(Attribute::NoBuiltin)) return 0;<br>- return Impl->optimizeCall(CI);<br>+ return Impl->optimizeCall(CI, TD);<br>}<br><br>void LibCallSimplifier::replaceAllUsesWith(Instruction *I, Value *With) const {<br>--<span class="Apple-converted-space"> </span><br>1.8.1</div></blockquote></div><br></body></html>