[PATCH] VirtRegRewriter compile time reduction.

[Quentin Colombet]

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