Skip to content

Commit 4ae9fa5

Browse files
aheejineddyb
authored andcommitted
[WebAssembly] Fix a non-determinism problem in FixIrreducibleControlFlow
Summary: We already sorted the blocks when fixing up a set of mutual loop entries, however, there can be multiple sets of such mutual loop entries, and the order we encounter them should not be random, so sort them too. Fixes https://bugs.llvm.org/show_bug.cgi?id=44982 Patch by Alon Zakai (kripken) Reviewers: aheejin, sbc100, dschuff Subscribers: mgrang, sunfish, hiraditya, jgravelle-google, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D74999
1 parent 6c040dd commit 4ae9fa5

File tree

1 file changed

+19
-9
lines changed

1 file changed

+19
-9
lines changed

llvm/lib/Target/WebAssembly/WebAssemblyFixIrreducibleControlFlow.cpp

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ namespace {
6666
using BlockVector = SmallVector<MachineBasicBlock *, 4>;
6767
using BlockSet = SmallPtrSet<MachineBasicBlock *, 4>;
6868

69+
static BlockVector getSortedEntries(const BlockSet &Entries) {
70+
BlockVector SortedEntries(Entries.begin(), Entries.end());
71+
llvm::sort(SortedEntries,
72+
[](const MachineBasicBlock *A, const MachineBasicBlock *B) {
73+
auto ANum = A->getNumber();
74+
auto BNum = B->getNumber();
75+
return ANum < BNum;
76+
});
77+
return SortedEntries;
78+
}
79+
6980
// Calculates reachability in a region. Ignores branches to blocks outside of
7081
// the region, and ignores branches to the region entry (for the case where
7182
// the region is the inner part of a loop).
@@ -241,20 +252,25 @@ class WebAssemblyFixIrreducibleControlFlow final : public MachineFunctionPass {
241252
bool WebAssemblyFixIrreducibleControlFlow::processRegion(
242253
MachineBasicBlock *Entry, BlockSet &Blocks, MachineFunction &MF) {
243254
bool Changed = false;
244-
245255
// Remove irreducibility before processing child loops, which may take
246256
// multiple iterations.
247257
while (true) {
248258
ReachabilityGraph Graph(Entry, Blocks);
249259

250260
bool FoundIrreducibility = false;
251261

252-
for (auto *LoopEntry : Graph.getLoopEntries()) {
262+
for (auto *LoopEntry : getSortedEntries(Graph.getLoopEntries())) {
253263
// Find mutual entries - all entries which can reach this one, and
254264
// are reached by it (that always includes LoopEntry itself). All mutual
255265
// entries must be in the same loop, so if we have more than one, then we
256266
// have irreducible control flow.
257267
//
268+
// (Note that we need to sort the entries here, as otherwise the order can
269+
// matter: being mutual is a symmetric relationship, and each set of
270+
// mutuals will be handled properly no matter which we see first. However,
271+
// there can be multiple disjoint sets of mutuals, and which we process
272+
// first changes the output.)
273+
//
258274
// Note that irreducibility may involve inner loops, e.g. imagine A
259275
// starts one loop, and it has B inside it which starts an inner loop.
260276
// If we add a branch from all the way on the outside to B, then in a
@@ -325,13 +341,7 @@ void WebAssemblyFixIrreducibleControlFlow::makeSingleEntryLoop(
325341
assert(Entries.size() >= 2);
326342

327343
// Sort the entries to ensure a deterministic build.
328-
BlockVector SortedEntries(Entries.begin(), Entries.end());
329-
llvm::sort(SortedEntries,
330-
[&](const MachineBasicBlock *A, const MachineBasicBlock *B) {
331-
auto ANum = A->getNumber();
332-
auto BNum = B->getNumber();
333-
return ANum < BNum;
334-
});
344+
BlockVector SortedEntries = getSortedEntries(Entries);
335345

336346
#ifndef NDEBUG
337347
for (auto Block : SortedEntries)

0 commit comments

Comments
 (0)