[llvm-commits] [llvm] r167759 - in /llvm/trunk: include/llvm/PassManagers.h lib/VMCore/PassManager.cpp
Chandler Carruth
chandlerc at google.com
Mon Nov 12 22:03:09 PST 2012
On Mon, Nov 12, 2012 at 7:18 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>
> On Nov 12, 2012, at 7:10 PM, Chandler Carruth <chandlerc at google.com> wrote:
>
>> On Mon, Nov 12, 2012 at 2:01 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>>>
>>> On 12.11.2012, at 22:42, Evan Cheng <evan.cheng at apple.com> wrote:
>>>
>>>> Author: evancheng
>>>> Date: Mon Nov 12 15:42:53 2012
>>>> New Revision: 167759
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=167759&view=rev
>>>> Log:
>>>> Cache size of PassVector to speed up getNumContainedPasses().
>>>> getNumContainedPasses() used to compute the size of the vector on demand. It is
>>>> called repeated in loops (such as runOnFunction()) and it can be updated while
>>>> inside the loop.
>>>
>>> Do you have some numbers to back this change? Getting the size of a small vector is loading two pointers, a sub and a shift. That shouldn't be expensive enough to justify adding more code.
>>>
>>> OTOH, the call to size() is inside a virtual function, and I don't think that any of the calls to it can be devirtualized. I'd expect the lack of inlining and the indirect call to be a lot slower than the arithmetic in size(). However, it looks like getNumContainedPasses() isn't overridden anywhere so maybe we should just make it non-virtual if it's popping up in profiles?
>>
>> As I've also been looking at both compile time and the pass managers,
>> I'd also love to know the rough scenario which causes this to show up
>> in profiles so I have a better idea of what might be important to
>> profile in making any changes here.
>
> I found it by inspection. Something like this is obvious inefficient.
> for (unsigned Index = 0; Index < getNumContainedPasses(); ++Index) {
>
> Unfortunately
> for (unsigned Index = 0, E < getNumContainedPasses(); Index < E; ++Index) {
>
> doesn't work because the number of pass vector may be updated during the loop. It's clear there are some design problems here.
>
> We've found pass manager overhead can add up to 10% of codegen time. That's insane. We'll be hopefully doing some work on this but this is just a low hanging fruit.
That's wild. 10%? Is there a test case (even a synthetic / contrived
one) that shows this problem?
Anyways, I'm looking at starting to work on the big pass manager work
I mentioned in an RFC email a few months ago, and so I'd like to watch
carefully any performance issues like this. Maybe I can even fix some
of them in the process if I get to stuff before you do (but who knows
if that'll happen...)
>
> Evan
>
>>
>>>
>>> - Ben
>>>
>>>>
>>>> Modified:
>>>> llvm/trunk/include/llvm/PassManagers.h
>>>> llvm/trunk/lib/VMCore/PassManager.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/PassManagers.h
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/PassManagers.h?rev=167759&r1=167758&r2=167759&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/PassManagers.h (original)
>>>> +++ llvm/trunk/include/llvm/PassManagers.h Mon Nov 12 15:42:53 2012
>>>> @@ -263,7 +263,7 @@
>>>> class PMDataManager {
>>>> public:
>>>>
>>>> - explicit PMDataManager() : TPM(NULL), Depth(0) {
>>>> + explicit PMDataManager() : TPM(NULL), PassVectorSize(0), Depth(0) {
>>>> initializeAnalysisInfo();
>>>> }
>>>>
>>>> @@ -344,7 +344,7 @@
>>>> void dumpPreservedSet(const Pass *P) const;
>>>>
>>>> virtual unsigned getNumContainedPasses() const {
>>>> - return (unsigned)PassVector.size();
>>>> + return PassVectorSize;
>>>> }
>>>>
>>>> virtual PassManagerType getPassManagerType() const {
>>>> @@ -369,14 +369,16 @@
>>>> // Top level manager.
>>>> PMTopLevelManager *TPM;
>>>>
>>>> - // Collection of pass that are managed by this manager
>>>> - SmallVector<Pass *, 16> PassVector;
>>>> -
>>>> // Collection of Analysis provided by Parent pass manager and
>>>> // used by current pass manager. At at time there can not be more
>>>> // then PMT_Last active pass mangers.
>>>> std::map<AnalysisID, Pass *> *InheritedAnalysis[PMT_Last];
>>>>
>>>> + // Collection of pass that are managed by this manager
>>>> + SmallVector<Pass *, 16> PassVector;
>>>> +
>>>> + // Cache the size of PassVector
>>>> + unsigned PassVectorSize;
>>>>
>>>> /// isPassDebuggingExecutionsOrMore - Return true if -debug-pass=Executions
>>>> /// or higher is specified.
>>>> @@ -444,7 +446,7 @@
>>>> }
>>>>
>>>> FunctionPass *getContainedPass(unsigned N) {
>>>> - assert ( N < PassVector.size() && "Pass number out of range!");
>>>> + assert ( N < PassVectorSize && "Pass number out of range!");
>>>> FunctionPass *FP = static_cast<FunctionPass *>(PassVector[N]);
>>>> return FP;
>>>> }
>>>>
>>>> Modified: llvm/trunk/lib/VMCore/PassManager.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/PassManager.cpp?rev=167759&r1=167758&r2=167759&view=diff
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/VMCore/PassManager.cpp (original)
>>>> +++ llvm/trunk/lib/VMCore/PassManager.cpp Mon Nov 12 15:42:53 2012
>>>> @@ -195,7 +195,7 @@
>>>> }
>>>>
>>>> BasicBlockPass *getContainedPass(unsigned N) {
>>>> - assert(N < PassVector.size() && "Pass number out of range!");
>>>> + assert(N < PassVectorSize && "Pass number out of range!");
>>>> BasicBlockPass *BP = static_cast<BasicBlockPass *>(PassVector[N]);
>>>> return BP;
>>>> }
>>>> @@ -346,7 +346,7 @@
>>>> }
>>>>
>>>> ModulePass *getContainedPass(unsigned N) {
>>>> - assert(N < PassVector.size() && "Pass number out of range!");
>>>> + assert(N < PassVectorSize && "Pass number out of range!");
>>>> return static_cast<ModulePass *>(PassVector[N]);
>>>> }
>>>>
>>>> @@ -963,6 +963,7 @@
>>>> if (!ProcessAnalysis) {
>>>> // Add pass
>>>> PassVector.push_back(P);
>>>> + ++PassVectorSize;
>>>> return;
>>>> }
>>>>
>>>> @@ -1024,6 +1025,7 @@
>>>>
>>>> // Add pass
>>>> PassVector.push_back(P);
>>>> + ++PassVectorSize;
>>>> }
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list