From 3687ac52a9fd6789a3088fd3eeca1e47ae04bfff Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Fri, 6 Jul 2018 14:20:58 +0000 Subject: [PATCH] [LoopSink] Make the enforcement of determinism deterministic. LoopBlockNumber is a DenseMap, comparing the result of find() will compare a pair. 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. llvm-svn: 336439 --- llvm/lib/Transforms/Scalar/LoopSink.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/LoopSink.cpp b/llvm/lib/Transforms/Scalar/LoopSink.cpp index 3cc53ea..760177c 100644 --- a/llvm/lib/Transforms/Scalar/LoopSink.cpp +++ b/llvm/lib/Transforms/Scalar/LoopSink.cpp @@ -202,15 +202,17 @@ static bool sinkInstruction(Loop &L, Instruction &I, 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()); -- 2.7.4