Skip to content

Commit 3bdf9a0

Browse files
fhahnfhahn
authored
[EquivalenceClasses] Use SmallVector for deterministic iteration order. (#134075)
Currently iterators over EquivalenceClasses will iterate over std::set, which guarantees the order specified by the comperator. Unfortunately in many cases, EquivalenceClasses are used with pointers, so iterating over std::set of pointers will not be deterministic across runs. There are multiple places that explicitly try to sort the equivalence classes before using them to try to get a deterministic order (LowerTypeTests, SplitModule), but there are others that do not at the moment and this can result at least in non-determinstic value naming in Float2Int. This patch updates EquivalenceClasses to keep track of all members via a extra SmallVector and removes code from LowerTypeTests and SplitModule to sort the classes before processing. Overall it looks like compile-time slightly decreases in most cases, but close to noise: https://llvm-compile-time-tracker.com/compare.php?from=7d441d9892295a6eb8aaf481e1715f039f6f224f&to=b0c2ac67a88d3ef86987e2f82115ea0170675a17&stat=instructions PR: #134075
1 parent 60efed3 commit 3bdf9a0

File tree

17 files changed

+86
-109
lines changed

17 files changed

+86
-109
lines changed

‎clang/lib/Analysis/FlowSensitive/SimplifyConstraints.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ void simplifyConstraints(llvm::SetVector<const Formula *> &Constraints,
111111
FalseAtoms = projectToLeaders(FalseAtoms, EquivalentAtoms);
112112

113113
llvm::DenseMap<Atom, const Formula *> Substitutions;
114-
for (auto It = EquivalentAtoms.begin(); It != EquivalentAtoms.end(); ++It) {
115-
Atom TheAtom = It->getData();
114+
for (const auto &E : EquivalentAtoms) {
115+
Atom TheAtom = E->getData();
116116
Atom Leader = EquivalentAtoms.getLeaderValue(TheAtom);
117117
if (TrueAtoms.contains(Leader)) {
118118
if (FalseAtoms.contains(Leader)) {
@@ -152,9 +152,9 @@ void simplifyConstraints(llvm::SetVector<const Formula *> &Constraints,
152152

153153
if (Info) {
154154
for (const auto &E : EquivalentAtoms) {
155-
if (!E.isLeader())
155+
if (!E->isLeader())
156156
continue;
157-
Atom At = *EquivalentAtoms.findLeader(E);
157+
Atom At = *EquivalentAtoms.findLeader(*E);
158158
if (TrueAtoms.contains(At) || FalseAtoms.contains(At))
159159
continue;
160160
llvm::SmallVector<Atom> Atoms =

‎llvm/include/llvm/ADT/EquivalenceClasses.h‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#ifndef LLVM_ADT_EQUIVALENCECLASSES_H
1616
#define LLVM_ADT_EQUIVALENCECLASSES_H
1717

18+
#include "llvm/ADT/SmallVector.h"
1819
#include <cassert>
1920
#include <cstddef>
2021
#include <cstdint>
@@ -138,6 +139,9 @@ class EquivalenceClasses {
138139
/// ECValues, it just keeps the key as part of the value.
139140
std::set<ECValue, ECValueComparator> TheMapping;
140141

142+
/// List of all members, used to provide a determinstic iteration order.
143+
SmallVector<const ECValue *> Members;
144+
141145
public:
142146
EquivalenceClasses() = default;
143147
EquivalenceClasses(const EquivalenceClasses &RHS) {
@@ -146,9 +150,10 @@ class EquivalenceClasses {
146150

147151
EquivalenceClasses &operator=(const EquivalenceClasses &RHS) {
148152
TheMapping.clear();
153+
Members.clear();
149154
for (const auto &E : RHS)
150-
if (E.isLeader()) {
151-
member_iterator MI = RHS.member_begin(E);
155+
if (E->isLeader()) {
156+
member_iterator MI = RHS.member_begin(*E);
152157
member_iterator LeaderIt = member_begin(insert(*MI));
153158
for (++MI; MI != member_end(); ++MI)
154159
unionSets(LeaderIt, member_begin(insert(*MI)));
@@ -161,19 +166,18 @@ class EquivalenceClasses {
161166
//
162167

163168
/// iterator* - Provides a way to iterate over all values in the set.
164-
using iterator =
165-
typename std::set<ECValue, ECValueComparator>::const_iterator;
169+
using iterator = typename SmallVector<const ECValue *>::const_iterator;
166170

167-
iterator begin() const { return TheMapping.begin(); }
168-
iterator end() const { return TheMapping.end(); }
171+
iterator begin() const { return Members.begin(); }
172+
iterator end() const { return Members.end(); }
169173

170174
bool empty() const { return TheMapping.empty(); }
171175

172176
/// member_* Iterate over the members of an equivalence class.
173177
class member_iterator;
174178
member_iterator member_begin(const ECValue &ECV) const {
175179
// Only leaders provide anything to iterate over.
176-
return member_iterator(ECV.isLeader() ? ECV.getLeader() : nullptr);
180+
return member_iterator(ECV.isLeader() ? &ECV : nullptr);
177181
}
178182

179183
member_iterator member_end() const {
@@ -208,7 +212,7 @@ class EquivalenceClasses {
208212
unsigned getNumClasses() const {
209213
unsigned NC = 0;
210214
for (const auto &E : *this)
211-
if (E.isLeader())
215+
if (E->isLeader())
212216
++NC;
213217
return NC;
214218
}
@@ -219,7 +223,10 @@ class EquivalenceClasses {
219223
/// insert - Insert a new value into the union/find set, ignoring the request
220224
/// if the value already exists.
221225
const ECValue &insert(const ElemTy &Data) {
222-
return *TheMapping.insert(ECValue(Data)).first;
226+
auto I = TheMapping.insert(ECValue(Data));
227+
if (I.second)
228+
Members.push_back(&*I.first);
229+
return *I.first;
223230
}
224231

225232
/// findLeader - Given a value in the set, return a member iterator for the

‎llvm/lib/Analysis/VectorUtils.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -844,10 +844,10 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
844844
DBits[ECs.getOrInsertLeaderValue(I.first)] |= ~0ULL;
845845

846846
for (const auto &E : ECs) {
847-
if (!E.isLeader())
847+
if (!E->isLeader())
848848
continue;
849849
uint64_t LeaderDemandedBits = 0;
850-
for (Value *M : make_range(ECs.member_begin(E), ECs.member_end()))
850+
for (Value *M : make_range(ECs.member_begin(*E), ECs.member_end()))
851851
LeaderDemandedBits |= DBits[M];
852852

853853
uint64_t MinBW = llvm::bit_width(LeaderDemandedBits);
@@ -859,15 +859,15 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
859859
// indvars.
860860
// If we are required to shrink a PHI, abandon this entire equivalence class.
861861
bool Abort = false;
862-
for (Value *M : make_range(ECs.member_begin(E), ECs.member_end()))
862+
for (Value *M : make_range(ECs.member_begin(*E), ECs.member_end()))
863863
if (isa<PHINode>(M) && MinBW < M->getType()->getScalarSizeInBits()) {
864864
Abort = true;
865865
break;
866866
}
867867
if (Abort)
868868
continue;
869869

870-
for (Value *M : make_range(ECs.member_begin(E), ECs.member_end())) {
870+
for (Value *M : make_range(ECs.member_begin(*E), ECs.member_end())) {
871871
auto *MI = dyn_cast<Instruction>(M);
872872
if (!MI)
873873
continue;

‎llvm/lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,9 @@ bool AArch64A57FPLoadBalancing::runOnBasicBlock(MachineBasicBlock &MBB) {
368368
// Convert the EquivalenceClasses to a simpler set of sets.
369369
std::vector<std::vector<Chain*> > V;
370370
for (const auto &E : EC) {
371-
if (!E.isLeader())
371+
if (!E->isLeader())
372372
continue;
373-
std::vector<Chain *> Cs(EC.member_begin(E), EC.member_end());
373+
std::vector<Chain *> Cs(EC.member_begin(*E), EC.member_end());
374374
if (Cs.empty()) continue;
375375
V.push_back(std::move(Cs));
376376
}

‎llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,11 +1017,12 @@ void RecursiveSearchSplitting::setupWorkList() {
10171017
}
10181018

10191019
for (const auto &Node : NodeEC) {
1020-
if (!Node.isLeader())
1020+
if (!Node->isLeader())
10211021
continue;
10221022

10231023
BitVector Cluster = SG.createNodesBitVector();
1024-
for (auto MI = NodeEC.member_begin(Node); MI != NodeEC.member_end(); ++MI) {
1024+
for (auto MI = NodeEC.member_begin(*Node); MI != NodeEC.member_end();
1025+
++MI) {
10251026
const SplitGraph::Node &N = SG.getNode(*MI);
10261027
if (N.isGraphEntryPoint())
10271028
N.getDependencies(Cluster);

‎llvm/lib/Transforms/IPO/LowerTypeTests.cpp‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2339,36 +2339,17 @@ bool LowerTypeTestsModule::lower() {
23392339
if (GlobalClasses.empty())
23402340
return false;
23412341

2342-
// Build a list of disjoint sets ordered by their maximum global index for
2343-
// determinism.
2344-
std::vector<std::pair<GlobalClassesTy::iterator, unsigned>> Sets;
2345-
for (GlobalClassesTy::iterator I = GlobalClasses.begin(),
2346-
E = GlobalClasses.end();
2347-
I != E; ++I) {
2348-
if (!I->isLeader())
2342+
// For each disjoint set we found...
2343+
for (const auto &C : GlobalClasses) {
2344+
if (!C->isLeader())
23492345
continue;
2350-
++NumTypeIdDisjointSets;
2351-
2352-
unsigned MaxUniqueId = 0;
2353-
for (GlobalClassesTy::member_iterator MI = GlobalClasses.member_begin(*I);
2354-
MI != GlobalClasses.member_end(); ++MI) {
2355-
if (auto *MD = dyn_cast_if_present<Metadata *>(*MI))
2356-
MaxUniqueId = std::max(MaxUniqueId, TypeIdInfo[MD].UniqueId);
2357-
else if (auto *BF = dyn_cast_if_present<ICallBranchFunnel *>(*MI))
2358-
MaxUniqueId = std::max(MaxUniqueId, BF->UniqueId);
2359-
}
2360-
Sets.emplace_back(I, MaxUniqueId);
2361-
}
2362-
llvm::sort(Sets, llvm::less_second());
23632346

2364-
// For each disjoint set we found...
2365-
for (const auto &S : Sets) {
2347+
++NumTypeIdDisjointSets;
23662348
// Build the list of type identifiers in this disjoint set.
23672349
std::vector<Metadata *> TypeIds;
23682350
std::vector<GlobalTypeMember *> Globals;
23692351
std::vector<ICallBranchFunnel *> ICallBranchFunnels;
2370-
for (GlobalClassesTy::member_iterator MI =
2371-
GlobalClasses.member_begin(*S.first);
2352+
for (GlobalClassesTy::member_iterator MI = GlobalClasses.member_begin(*C);
23722353
MI != GlobalClasses.member_end(); ++MI) {
23732354
if (isa<Metadata *>(*MI))
23742355
TypeIds.push_back(cast<Metadata *>(*MI));

‎llvm/lib/Transforms/Scalar/Float2Int.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,14 +312,16 @@ bool Float2IntPass::validateAndTransform(const DataLayout &DL) {
312312

313313
// Iterate over every disjoint partition of the def-use graph.
314314
for (const auto &E : ECs) {
315-
if (!E.isLeader())
315+
if (!E->isLeader())
316316
continue;
317+
317318
ConstantRange R(MaxIntegerBW + 1, false);
318319
bool Fail = false;
319320
Type *ConvertedToTy = nullptr;
320321

321322
// For every member of the partition, union all the ranges together.
322-
for (auto MI = ECs.member_begin(E), ME = ECs.member_end(); MI != ME; ++MI) {
323+
for (auto MI = ECs.member_begin(*E), ME = ECs.member_end(); MI != ME;
324+
++MI) {
323325
Instruction *I = *MI;
324326
auto SeenI = SeenInsts.find(I);
325327
if (SeenI == SeenInsts.end())
@@ -349,7 +351,7 @@ bool Float2IntPass::validateAndTransform(const DataLayout &DL) {
349351

350352
// If the set was empty, or we failed, or the range is poisonous,
351353
// bail out.
352-
if (ECs.member_begin(E) == ECs.member_end() || Fail || R.isFullSet() ||
354+
if (ECs.member_begin(*E) == ECs.member_end() || Fail || R.isFullSet() ||
353355
R.isSignWrappedSet())
354356
continue;
355357
assert(ConvertedToTy && "Must have set the convertedtoty by this point!");
@@ -389,7 +391,7 @@ bool Float2IntPass::validateAndTransform(const DataLayout &DL) {
389391
}
390392
}
391393

392-
for (auto MI = ECs.member_begin(E), ME = ECs.member_end(); MI != ME; ++MI)
394+
for (auto MI = ECs.member_begin(*E), ME = ECs.member_end(); MI != ME; ++MI)
393395
convert(*MI, Ty);
394396
MadeChange = true;
395397
}

‎llvm/lib/Transforms/Scalar/LoopDistribute.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -386,11 +386,11 @@ class InstPartitionContainer {
386386
// Merge the member of an equivalence class into its class leader. This
387387
// makes the members empty.
388388
for (const auto &C : ToBeMerged) {
389-
if (!C.isLeader())
389+
if (!C->isLeader())
390390
continue;
391391

392-
auto PartI = C.getData();
393-
for (auto *PartJ : make_range(std::next(ToBeMerged.member_begin(C)),
392+
auto PartI = C->getData();
393+
for (auto *PartJ : make_range(std::next(ToBeMerged.member_begin(*C)),
394394
ToBeMerged.member_end())) {
395395
PartJ->moveTo(*PartI);
396396
}

‎llvm/lib/Transforms/Utils/SplitModule.cpp‎

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -170,40 +170,24 @@ static void findPartitions(Module &M, ClusterIDMapType &ClusterIDMap,
170170
for (unsigned i = 0; i < N; ++i)
171171
BalancingQueue.push(std::make_pair(i, 0));
172172

173-
using SortType = std::pair<unsigned, ClusterMapType::iterator>;
174-
175-
SmallVector<SortType, 64> Sets;
176173
SmallPtrSet<const GlobalValue *, 32> Visited;
177174

178175
// To guarantee determinism, we have to sort SCC according to size.
179176
// When size is the same, use leader's name.
180-
for (ClusterMapType::iterator I = GVtoClusterMap.begin(),
181-
E = GVtoClusterMap.end();
182-
I != E; ++I)
183-
if (I->isLeader())
184-
Sets.push_back(
185-
std::make_pair(std::distance(GVtoClusterMap.member_begin(*I),
186-
GVtoClusterMap.member_end()),
187-
I));
188-
189-
llvm::sort(Sets, [](const SortType &a, const SortType &b) {
190-
if (a.first == b.first)
191-
return a.second->getData()->getName() > b.second->getData()->getName();
192-
else
193-
return a.first > b.first;
194-
});
195-
196-
for (auto &I : Sets) {
177+
for (const auto &C : GVtoClusterMap) {
178+
if (!C->isLeader())
179+
continue;
180+
197181
unsigned CurrentClusterID = BalancingQueue.top().first;
198182
unsigned CurrentClusterSize = BalancingQueue.top().second;
199183
BalancingQueue.pop();
200184

201185
LLVM_DEBUG(dbgs() << "Root[" << CurrentClusterID << "] cluster_size("
202-
<< I.first << ") ----> " << I.second->getData()->getName()
203-
<< "\n");
186+
<< std::distance(GVtoClusterMap.member_begin(*C),
187+
GVtoClusterMap.member_end())
188+
<< ") ----> " << C->getData()->getName() << "\n");
204189

205-
for (ClusterMapType::member_iterator MI =
206-
GVtoClusterMap.findLeader(*I.second);
190+
for (ClusterMapType::member_iterator MI = GVtoClusterMap.findLeader(*C);
207191
MI != GVtoClusterMap.member_end(); ++MI) {
208192
if (!Visited.insert(*MI).second)
209193
continue;

‎llvm/test/CodeGen/WebAssembly/cfi.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ define void @h() !type !0 {
1414
}
1515

1616
; CHECK-LABEL: f:
17-
; CHECK: .indidx 1
17+
; CHECK: .indidx 2
1818
define void @f() !type !0 {
1919
ret void
2020
}
2121

2222
; CHECK-LABEL: g:
23-
; CHECK: .indidx 2
23+
; CHECK: .indidx 1
2424
define void @g() !type !1 {
2525
ret void
2626
}

0 commit comments

Comments
 (0)