[PATCH] D45138: [MC] Change std::sort to llvm::sort in response to r327219
Grang, Mandeep Singh via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 12 13:57:22 PDT 2018
Actually, I would prefer them to be separate. Makes it easier to track
builtbot breakage and revert if needed. Also since all the other llvm
patches to replace std::sort to llvm::sort have been small target/area
specific I would like to follow the same for these four.
Thanks,
Mandeep
On 4/12/2018 1:52 PM, Rui Ueyama wrote:
> Could you just merge these four changes in a single patch?
>
> On Thu, Apr 12, 2018 at 1:51 PM Rui Ueyama <ruiu at google.com
> <mailto:ruiu at google.com>> wrote:
>
> Since these are mechanical changes, I'll review and give LGTM.
>
> On Thu, Apr 12, 2018 at 1:49 PM Grang, Mandeep Singh
> <mgrang at codeaurora.org <mailto:mgrang at codeaurora.org>> wrote:
>
> @ruiu @rksimon These 4 are the last remaining patches pending
> review and merge. Is it possible for you to review them?
> https://reviews.llvm.org/D45142
> https://reviews.llvm.org/D45139
> https://reviews.llvm.org/D45138
> https://reviews.llvm.org/D45137
>
> --Mandeep
>
> On 4/9/2018 3:34 PM, Grang, Mandeep Singh wrote:
>>
>> On 4/9/2018 3:16 PM, Rui Ueyama wrote:
>>> On Mon, Apr 9, 2018 at 3:13 PM Mandeep Singh Grang via
>>> Phabricator <reviews at reviews.llvm.org
>>> <mailto:reviews at reviews.llvm.org>> wrote:
>>>
>>> mgrang added a comment.
>>>
>>> In https://reviews.llvm.org/D45138#1062213, @ruiu wrote:
>>>
>>> > I'm not familiar with r327219, but if a decision to
>>> use randomized sort has been made, why don't you replace
>>> all occurrences of std::sort with llvm::sort in a single
>>> patch? I don't think that an author of each file don't
>>> have to understand this kind of change and approve
>>> individually.
>>>
>>>
>>> I had a patch to replace *all* occurrences of std::sort
>>> to llvm::sort https://reviews.llvm.org/D44363. However,
>>> the reviewers felt that the patch was getting too big
>>> and suggested splitting it into smaller patches
>>> target-wise, tablegen, MC, etc. Hence these separate
>>> patches.
>>>
>>>
>>> If the only reason not to submit them as a single patch is
>>> its size, I can actually approve. As long as it is a
>>> mechanical patch, I'm not worried too much about its size.
>> Adding @rksimon since he has been reviewing my patches. Is it
>> OK if I squash the remaining 4 patches
>> (D45142D45139D45138D45137) into one and push them as a single
>> patch?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180412/33511a9a/attachment.html>
More information about the llvm-commits
mailing list