[PATCH] ADT SortedVector and MachineBasicBlock::LiveIns changes.

Puyan Lotfi plotfi at apple.com
Wed Jul 22 16:40:50 PDT 2015


Thanks Quentin!

> On Jul 15, 2015, at 3:31 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi Puyan,
> 
> Thanks for working on this!
> 
> A couple of high level questions/remarks:
> What are the compile time figures with those patches?
> 

I’ve not measured yet; do you think I should necesarily do this prior to submitting?
Based on the size of the LiveIns vectors I’ve observed, I doubt this is something I need to do immediately. 

> I think the SortedVector should have a Compare functor to be part of the ADT.
> 

Done.

> 
> +  // Sort and Unique the LiveIns just in case.
> +  MBB->sortUniqueLiveIns();
> +
> 
> Shouldn’t this be a an invariant of the BasicBlock?
> Have you seen that being false at some point? (Other than when we are building the sets?)
> 

Sorry, this was accidentally leftover. Removed, and added the proper sortUniqueLiveIns call in one more place where we do an insert.

> 
> Nitpick:
> +  bool verify() const {
> +    for (unsigned i = 1; i < Vector.size(); i++)
> +      if (Vector[i - 1] >= Vector[i])
> +        return false;
> +    return true;
> +  }
> 
> Shouldn’t this check occur only when IsSorted is true or at least check that IsSorted is true?
> 

Yeah, this is simpler. I wanted something that was more of a brute force check for verify but I prefer the simplicity. 

> Thanks,
> -Quentin
> 

Newer patches based on these suggestions:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: SortedVector_Target.patch
Type: application/octet-stream
Size: 6026 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150722/281c7ac2/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SortedVector_CodeGen.patch
Type: application/octet-stream
Size: 2919 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150722/281c7ac2/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SortedVector_ADT.patch
Type: application/octet-stream
Size: 3882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150722/281c7ac2/attachment-0002.obj>
-------------- next part --------------


PL


> 
>> On Jul 15, 2015, at 1:58 PM, Puyan Lotfi <plotfi at apple.com> wrote:
>> 
>> All:
>> 
>> The following patches include a new ADT I’d like to add to LLVM: the SortedVector.
>> SortedVector is a std::vector that enforces sorted ordering (ascending for now) and uniqueness on access but allows for insertions to be unsorted. 
>> 
>> The reason I implemented this is because I noticed that MachineBasicBlock LiveIns are not actually sorted or unique even though they are normally almost sorted and almost unique.
>> So by implementing this ADT and by also adding a machine verifier check they will actually be sorted and unique on access. 
>> 
>> I’ve split my patch into:
>> 
>> - The SortedVector ADT
>> - Changes to MachineBasicBlock
>> - Changes to files in lib/CodeGen
>> - Changes to the targets
>> - An added machine verifier check
>> 
>> Most of the changes to lib/CodeGen and the Targets are just calls to the sorting method usually after a loop where many LiveIns are added to the MachineBasicBlock::LiveIns vector. 
>> 
>> Thanks
>> 
>> PL
>> 
>> 
>> <SortedVector_ADT.patch><SortedVector_CodeGen.patch><SortedVector_MachineBasicBlock.patch><SortedVector_MachineVerifier.patch><SortedVector_Target.patch>_______________________________________________
>> 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