[llvm] r303850 - [GVNSink] GVNSink pass

[Galina Kistanova via llvm-commits]

Hi James.

Of course. No worries.

I'm going to change that comparison operator to return A > B, and will
disable the test that would be failing because of that change.

Good luck with the job transition.

Thanks

Galina



On Wed, Jun 7, 2017 at 2:06 AM, James Molloy <james at jamesmolloy.co.uk>
wrote:

> Hi Galina,
>
> Apologies, I am between jobs at the moment and do not have access to any
> machine that can build LLVM. I will have access to one on Monday evening
> when I start with Google.
>
> In the meantime perhaps you could commit the minimal change to make the
> bot green and I could check it / handle the fallout on Monday?
>
> James
>
> On Wed, 7 Jun 2017 at 03:15, Galina Kistanova <gkistanova at gmail.com>
> wrote:
>
>> Hello James.
>>
>> Here is the bug:
>>
>>
>> +  std::stable_sort(
>> +      Candidates.begin(), Candidates.end(),
>> +      [](const SinkingInstructionCandidate &A,
>> +         const SinkingInstructionCandidate &B) { return A >= B; });
>>
>> This comparison operator is invalid, because it does not implement strict
>> weak ordering. With this logic, if 2 candidates A and B have the same cost,
>> the comparison operator will return true, but must return false.
>>
>> This fix would fix the http://lab.llvm.org:8011/
>> builders/llvm-clang-x86_64-expensive-checks-win bot and
>> https://bugs.llvm.org/show_bug.cgi?id=33210.
>>
>> Could you take care of this, please?
>>
>> Thanks
>>
>> Galina
>>
>>
>> On Fri, Jun 2, 2017 at 12:18 PM, Galina Kistanova <gkistanova at gmail.com>
>> wrote:
>>
>>> The http://lab.llvm.org:8011/builders/llvm-clang-x86_64-
>>> expensive-checks-win bot builds the debug configuration now.
>>>
>>> James, could you fix the GVNSink.cpp, line 749, please?
>>>
>>> DEBUG(dbgs() << " -- Sinking candidates:\n"; for (auto &C
>>>                                                     : Candidates) dbgs()
>>>                                                << "  " << C << "\n";);
>>> It triggers a crash.
>>> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-
>>> expensive-checks-win/builds/2891/steps/test-check-all/logs/stdio
>>>
>>> Thanks
>>>
>>> Galina
>>>
>>>
>>> On Tue, May 30, 2017 at 12:14 PM, Davide Italiano <davide at freebsd.org>
>>> wrote:
>>>
>>>> On Tue, May 30, 2017 at 11:34 AM, Galina Kistanova <
>>>> gkistanova at gmail.com> wrote:
>>>> > I'll see if I could find few spare CPU circles on one of the MSVC
>>>> builder.
>>>> >
>>>> > What targets we are after here? Just LLVM or LLVM+Clang?
>>>> > If this is LLVM only, I might be able to reconfigure one of the
>>>> existing
>>>> > builders to build debug instead of the release.
>>>> >
>>>>
>>>> I think the crash was triggered by LLVM, but having LLVM + clang would
>>>> be better.
>>>>
>>>> --
>>>> Davide
>>>>
>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170607/a1424d0c/attachment.html>