[PATCH] VirtRegRewriter compile time reduction.

Puyan Lotfi plotfi at apple.com
Thu May 21 15:52:27 PDT 2015


I’ll commit to working on that; you have my word.
So I’ll check this in soon; thanks Quentin.

PL


> On May 21, 2015, at 2:07 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi Puyan,
> 
> The change itself looks good, but I would add a check in the machine verifier to be sure we do not forget about the uniqueness property.
> 
> In other words, could you propose a patch for the MachineVerifier to check that each register appears at most once.
> This can do as a follow-up patch as long as you commit to it :)
> 
> Cheers,
> -Quentin
> 
>> On May 21, 2015, at 1:56 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>> 
>> Added feedback changes.
>> 
>> PL
>> 
>> <VirtRegMapCompileTime2.patch>
>> 
>>> On May 21, 2015, at 11:46 AM, Matthias Braun <mbraun at apple.com> wrote:
>>> 
>>> Hi Puyan,
>>> 
>>> LGTM with the following nitpicks fixed:
>>> 
>>> +  /// addLiveIn - Add the specified register as a live in. Note that it is
>>> 
>>> +  /// sortUniqueLiveIns - Sorts and uniques the LiveIns vector. It can be
>>> 
>>> Don't repeat the method name in the documentation comment.
>>> 
>>> +  for (auto MBBI = MF->begin(), MBBE = MF->end(); MBBI != MBBE; ++MBBI)
>>> A range based for loop and the actual type instead of "auto" is better for readers of the code.
>>> 
>>> - Matthias
>>> 
>>> 
>>>> On May 21, 2015, at 9:59 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Your patch does not pass clang-format (80 columns limit for example).
>>>> It also seems to me that the loop you are adding could be a C++11 for-range loop.
>>>> 
>>>>>>>> Mehdi
>>>> 
>>>>> On May 21, 2015, at 12:54 AM, Puyan Lotfi <plotfi at apple.com> wrote:
>>>>> 
>>>>> Hi
>>>>> 
>>>>> I have a patch (attached) for reducing compile time in VirtRegRewriter
>>>>> (especially for targets with a very high number of registers).
>>>>> 
>>>>> The patch changes how VirtRegRewriter::addMBBLiveIns adds registers to
>>>>> each MachineBasicBlock's LiveIns vector by adding registers to
>>>>> MachineBasicBlock::LiveIns without a isLiveIn check (the vector should
>>>>> be sorted and not have duplicates), and after all registers are added
>>>>> the LiveIns vector for each MachineBasicBlock for the current
>>>>> MachineFunction is sorted and uniqued (a call to sortUniqueLiveIns).
>>>>> This improves compile time by avoiding the O(LiveIns.size()) checks at
>>>>> every addLiveIn attempt which can get expensive if MRI->getNumVirtRegs()
>>>>> is high as this is number of iterations of the outer loop.
>>>>> 
>>>>> I'd like any feedback.
>>>>> 
>>>>> Thanks
>>>>> 
>>>>> PL
>>>>> 
>>>>> <VirtRegRewriterCompileTime.patch>_______________________________________________
>>>>> 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
>>> 
>> 
>> _______________________________________________
>> 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