[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