Hi Geoff,<br><br>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.<br><br>Cheers,<br><br>James<br><div class="gmail_quote">On Fri, 30 Jan 2015 at 03:51, Geoff Berry <<a href="mailto:gberry@codeaurora.org">gberry@codeaurora.org</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi jmolloy, mcrosier, apazos,<br>
<br>
Add tie breaker to colorChainSet() sort so that processing order doesn't<br>
depend on std::set order, which depends on pointer order, which is<br>
unstable from run to run.<br>
<br>
REPOSITORY<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D7265" target="_blank">http://reviews.llvm.org/D7265</a><br>
<br>
Files:<br>
lib/Target/AArch64/<u></u>AArch64A57FPLoadBalancing.cpp<br>
<br>
Index: lib/Target/AArch64/<u></u>AArch64A57FPLoadBalancing.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- lib/Target/AArch64/<u></u>AArch64A57FPLoadBalancing.cpp<br>
+++ lib/Target/AArch64/<u></u>AArch64A57FPLoadBalancing.cpp<br>
@@ -259,7 +259,7 @@<br>
}<br>
<br>
/// Return true if this chain starts before Other.<br>
- bool startsBefore(Chain *Other) {<br>
+ bool startsBefore(const Chain *Other) const {<br>
return StartInstIdx < Other->StartInstIdx;<br>
}<br>
<br>
@@ -431,10 +431,17 @@<br>
// chains that we cannot change before we look at those we can,<br>
// so the parity counter is updated and we know what color we should<br>
// change them to!<br>
+ // Final tie-break with instruction order so pass output is stable (i.e. not<br>
+ // dependent on malloc'd pointer values).<br>
std::sort(GV.begin(), GV.end(), [](const Chain *G1, const Chain *G2) {<br>
if (G1->size() != G2->size())<br>
return G1->size() > G2->size();<br>
- return G1->requiresFixup() > G2->requiresFixup();<br>
+ if (G1->requiresFixup() != G2->requiresFixup())<br>
+ return G1->requiresFixup() > G2->requiresFixup();<br>
+ // Make sure startsBefore() produces a stable final order.<br>
+ assert((G1 == G2 || (G1->startsBefore(G2) ^ G2->startsBefore(G1))) &&<br>
+ "Starts before not total order!");<br>
+ return G1->startsBefore(G2);<br>
});<br>
<br>
Color PreferredColor = Parity < 0 ? Color::Even : Color::Odd;<br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>