[llvm] r336439 - [LoopSink] Make the enforcement of determinism deterministic.

[Benjamin Kramer via llvm-commits]

Author: d0k
Date: Fri Jul  6 07:20:58 2018
New Revision: 336439

URL: http://llvm.org/viewvc/llvm-project?rev=336439&view=rev
Log:
[LoopSink] Make the enforcement of determinism deterministic.

LoopBlockNumber is a DenseMap<BasicBlock*, int>, comparing the result of
find() will compare a pair<BasicBlock*, int>. That's of course depending
on pointer ordering which varies from run to run. Reverse iteration
doesn't find this because we're copying to a vector first.

This bug has been there since 2016 but only recently showed up on clang
selfhost with FDO and ThinLTO, which is also why I didn't manage to get
a reasonable test case for this. Add an assert that would've caught
this.

Modified:
    llvm/trunk/lib/Transforms/Scalar/LoopSink.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/LoopSink.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopSink.cpp?rev=336439&r1=336438&r2=336439&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/LoopSink.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/LoopSink.cpp Fri Jul  6 07:20:58 2018
@@ -202,15 +202,17 @@ static bool sinkInstruction(Loop &L, Ins
                              BBsToSinkInto.end());
   llvm::sort(SortedBBsToSinkInto.begin(), SortedBBsToSinkInto.end(),
              [&](BasicBlock *A, BasicBlock *B) {
-               return *LoopBlockNumber.find(A) < *LoopBlockNumber.find(B);
+               return LoopBlockNumber.find(A)->second <
+                      LoopBlockNumber.find(B)->second;
              });
 
   BasicBlock *MoveBB = *SortedBBsToSinkInto.begin();
   // FIXME: Optimize the efficiency for cloned value replacement. The current
   //        implementation is O(SortedBBsToSinkInto.size() * I.num_uses()).
-  for (BasicBlock *N : SortedBBsToSinkInto) {
-    if (N == MoveBB)
-      continue;
+  for (BasicBlock *N : makeArrayRef(SortedBBsToSinkInto).drop_front(1)) {
+    assert(LoopBlockNumber.find(N)->second >
+               LoopBlockNumber.find(MoveBB)->second &&
+           "BBs not sorted!");
     // Clone I and replace its uses.
     Instruction *IC = I.clone();
     IC->setName(I.getName());