[llvm] r318743 - SLPVectorizer.cpp: Avoid std::stable_sort(properlyDominates()).

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 2 14:53:00 PST 2018


Given there's been no response from the patch author to substantial 
post-commit review comment, I think it would be entirely appropriate to 
revert.  I'll defer to you as to whether you actually want to do it, but 
it seems called for to me.

Philip

On 01/02/2018 02:36 PM, Davide Italiano wrote:
> I don't think the alternative solution was ever committed.
>
> On Tue, Jan 2, 2018 at 6:28 PM, Philip Reames <listmail at philipreames.com> wrote:
>> On 11/26/2017 05:15 PM, Davide Italiano via llvm-commits wrote:
>>> On Tue, Nov 21, 2017 at 9:54 PM, Davide Italiano <davide at freebsd.org>
>>> wrote:
>>>> On Tue, Nov 21, 2017 at 1:41 AM, NAKAMURA Takumi via llvm-commits
>>>> <llvm-commits at lists.llvm.org> wrote:
>>>>> Author: chapuni
>>>>> Date: Tue Nov 21 01:41:01 2017
>>>>> New Revision: 318743
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=318743&view=rev
>>>>> Log:
>>>>> SLPVectorizer.cpp: Avoid std::stable_sort(properlyDominates()).
>>>>>
>>>>> properlyDominates() shouldn't be used as sort key. It causes different
>>>>> output between stdlibc++ and libc++.
>>>>> Instead, I introduced RPOT. In most cases, it works for CSE.
>>>>>
>>>> Why using the dominator DFSIn/DFSOut numbering not enough? I'm not
>>>> sure you need reverse post order here.
>>>> [I thought this change was never approved, but I might have missed
>>>> something]
>>>>
>>>> Thanks!
>>>>
>>>> --
>>>> Davide
>>> ahem ... ping? I'm a little worried because this wasn't really
>>> reviewed and you went ahead and committed a patch which uses a
>>> different approach than the one proposed.
>> Was there resolution on this I haven't seen?  If not, we should revert the
>> patch due to non-response.
>>
>>
>
>



More information about the llvm-commits mailing list