[PATCH] AArch64: Make AArch64A57FPLoadBalancing output stable.

[Chad Rosier]

Hi James,
In discussing this with Geoff, I believe the underlying issue is that
we're sorting based on pointer values, which change from run to run.  This
is what is introducing the non-determinism.

IIRC, the difference between std::sort and std::stable_sort is that
std::stable_sort guarantees the order of non-unique elements to be
preserved.  Sanjin and Geoff have both indicated to me that the elements
in the vector are unique (I didn't verify), so std::sort wouldn't really
the problem in this case.

Geoff/James, feel free to correct me if I misspoke.

 Chad

> Hi Geoff,
>
> Thanks for fixing this! It looks ok as-is, but wouldn't switching simply
> from std::sort to std::stable_sort solve the problem easier? Stable_sort
> is
> used for this reason in other areas of the compiler.
>
> Cheers,
>
> James
> On Fri, 30 Jan 2015 at 03:51, Geoff Berry <gberry at codeaurora.org> wrote:
>
>> Hi jmolloy, mcrosier, apazos,
>>
>> Add tie breaker to colorChainSet() sort so that processing order doesn't
>> depend on std::set order, which depends on pointer order, which is
>> unstable from run to run.
>>
>> REPOSITORY
>>   rL LLVM
>>
>> http://reviews.llvm.org/D7265
>>
>> Files:
>>   lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp
>>
>> Index: lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp
>> ===================================================================
>> --- lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp
>> +++ lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp
>> @@ -259,7 +259,7 @@
>>    }
>>
>>    /// Return true if this chain starts before Other.
>> -  bool startsBefore(Chain *Other) {
>> +  bool startsBefore(const Chain *Other) const {
>>      return StartInstIdx < Other->StartInstIdx;
>>    }
>>
>> @@ -431,10 +431,17 @@
>>    // chains that we cannot change before we look at those we can,
>>    // so the parity counter is updated and we know what color we should
>>    // change them to!
>> +  // Final tie-break with instruction order so pass output is stable
>> (i.e. not
>> +  // dependent on malloc'd pointer values).
>>    std::sort(GV.begin(), GV.end(), [](const Chain *G1, const Chain *G2)
>> {
>>        if (G1->size() != G2->size())
>>          return G1->size() > G2->size();
>> -      return G1->requiresFixup() > G2->requiresFixup();
>> +      if (G1->requiresFixup() != G2->requiresFixup())
>> +        return G1->requiresFixup() > G2->requiresFixup();
>> +      // Make sure startsBefore() produces a stable final order.
>> +      assert((G1 == G2 || (G1->startsBefore(G2) ^
>> G2->startsBefore(G1)))
>> &&
>> +             "Starts before not total order!");
>> +      return G1->startsBefore(G2);
>>      });
>>
>>    Color PreferredColor = Parity < 0 ? Color::Even : Color::Odd;
>>
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>