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

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 7 14:07:55 PST 2018


Thanks.

commit e7e645cee5c4c58d509f00bfd1895d7a6f37b7bc (HEAD -> master,
origin/master, origin/HEAD)
Author: davide <davide at 91177308-0d34-0410-b5e6-96231b3b80d8>
Date:   Sun Jan 7 22:06:24 2018 +0000

    [SLPVectorizer] Reintroduce std::stable_sort(properlyDominates()).

On Tue, Jan 2, 2018 at 2:53 PM, Philip Reames <listmail at philipreames.com> wrote:
> 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.
>>>
>>>
>>
>>
>



-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare


More information about the llvm-commits mailing list