[PATCH] Compile time reduction for small programs.

Puyan Lotfi plotfi at apple.com
Wed Feb 5 12:34:18 PST 2014


On Feb 5, 2014, at 9:46 AM, Eric Christopher <echristo at gmail.com> wrote:

> On Wed, Feb 5, 2014 at 9:39 AM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:
>> 
>> On Feb 4, 2014, at 11:41 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 
>>> On Tue, Feb 4, 2014 at 10:26 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>>>> 
>>>> On Feb 4, 2014, at 10:16 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>> 
>>>>> On Tue, Feb 4, 2014 at 9:47 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>>>>>> 
>>>>>> On Feb 4, 2014, at 6:52 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>>>> 
>>>>>>> Hey cool,
>>>>>>> 
>>>>>>> +/// reinitPhysRegEntries - Initializes PhysRegEntries to new
>>>>>>> +/// zeroed out memory of size NumPhysRegs. This is faster than using
>>>>>>> +/// llvm::SmallVector::assign(N, 0). It's used in the below code to
>>>>>>> +/// also supports when a pass manager could be reused for different
>>>>>>> +/// but similar targets that could have varying numbers of PhysRegs.
>>>>>>> +void InterferenceCache::reinitPhysRegEntries() {
>>>>>>> +  if (PhysRegEntriesCount == TRI->getNumRegs()) return;
>>>>>>> +  free(PhysRegEntries);
>>>>>>> +  PhysRegEntriesCount = TRI->getNumRegs();
>>>>>>> +  PhysRegEntries = (unsigned char*) calloc(PhysRegEntriesCount,
>>>>>>> sizeof(unsigned char));
>>>>>>> +}
>>>>>>> 
>>>>>>> Looks to be faster since we're not actually zeroing out the array if
>>>>>>> it's the same size as the previous allocation?
>>>>>>> 
>>>>>> 
>>>>>> Not exactly; the compile time improvement gained here over the current implementation comes from the fact that we're using a SmallVector:
>>>>>> 
>>>>>> SmallVector<unsigned char, 2> PhysRegEntries;
>>>>>> 
>>>>>> that we zero out using assign:
>>>>>> 
>>>>>> PhysRegEntries.assign(TRI->getNumRegs(), 0);
>>>>>> 
>>>>>> when the SmallVector might not actually be that small at all.
>>>>>> 
>>>>>> Compile time was being spent in zeroing out the SmallVector when calling llvm::SmallVector::assign, which uses an iterator to assign each element in the SmallVector to zero.
>>>>>> I found that just by changing the SmallVector to a std::vector I got a compile time reduction since std::vector ends up using __platform_bzero to do the job.
>>>>>> 
>>>>>> Jakob suggested I just calloc the memory in the first place, rather than using a std::vector or calling bzero.
>>>>>> And actually, it would be nice if SmallVector had some platform specific speed improvements eventually, the way that std::vector does.
>>>>>> 
>>>>> 
>>>>> Yeah, it would be. That's a bit crazy.
>>>>> 
>>>>> At any rate, I was wondering why reinit didn't 0 the mem if it
>>>>> happened to be the same size?
>>>>> 
>>>> 
>>>> Hmm, I am actually not sure what is correct in this case. I'll have to ask Jakob.
>>>> But the reinit here is more or less a precaution.
>>>> I believe the existing code would have used the same already populated SmallVector memory if the pass manager we to be reused too.
>>>> 
>>> 
>>> Right, I'm just confused as to the seeming (?) assumption that a
>>> reused pass manager with a different machine would have the same
>>> registers in the phys reg map? Or maybe I'm confused :)
>> 
>> Looking at the top of InterferenceCache::get(), the PhysRegEntries array is used in the same way as the sparse array in llvm::SparseSet, see also Preston's paper: http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.30.7319
>> 
>> The double-checking "E < CacheEntries && Entries[E].getPhysReg() == PhysReg" means that the initial values of the array don't matter, and so the only reason for clearing it is to shut up tools like valgrind. That's why it can simply be allocated with calloc() and not cleared in reinitPhysRegEntries().
>> 
>> Clearly, this needs to be explained in a comment if we're going to bypass the zeroing.
>> 
> 
> Aha. That makes total sense. And please :)
> 
> -eric
> 

Zeroing explanation added:



Eric, Jakob: does this patch now look good to you?

-Puyan

>> Thanks,
>> /jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/a8f62417/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: CompileTimeReductionSmall2.patch
Type: application/octet-stream
Size: 6954 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/a8f62417/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140205/a8f62417/attachment-0001.html>


More information about the llvm-commits mailing list