Skip to content

[EquivalenceClasses] Use SmallVector for deterministic iteration order. #134075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 2, 2025
Merged

[EquivalenceClasses] Use SmallVector for deterministic iteration order. #134075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
10afe2f
[EquivalenceClasses] Use SmallVector for deterministic iteration order.
fhahn Mar 29, 2025
3e50db9
!fixup update missed tests.
fhahn Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/lib/Analysis/FlowSensitive/SimplifyConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ void simplifyConstraints(llvm::SetVector<const Formula *> &Constraints,
FalseAtoms = projectToLeaders(FalseAtoms, EquivalentAtoms);

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

if (Info) {
for (const auto &E : EquivalentAtoms) {
if (!E.isLeader())
if (!E->isLeader())
continue;
Atom At = *EquivalentAtoms.findLeader(E);
Atom At = *EquivalentAtoms.findLeader(*E);
if (TrueAtoms.contains(At) || FalseAtoms.contains(At))
continue;
llvm::SmallVector<Atom> Atoms =
Expand Down
25 changes: 16 additions & 9 deletions llvm/include/llvm/ADT/EquivalenceClasses.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef LLVM_ADT_EQUIVALENCECLASSES_H
#define LLVM_ADT_EQUIVALENCECLASSES_H

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

/// List of all members, used to provide a determinstic iteration order.
SmallVector<const ECValue *> Members;
Comment on lines +142 to +143
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you combine into SetVector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where things get a little tricky. ECValue contain pointers to other ECValue's (the leader of the class and the next member of the class), so using something like SetVector as the backing storage won't work I think, as it might reallocate and move the objects.

One thing I tried is using a BumpAllocator for the elements in the class and a separate DenseMap instead of the std::set: 5c13864

I think SetVector (or MapVector) would be slightly worse here, as it stores an ECValue in both a vector and a denseSet, and ECValue contains 3 pointers, so it would require quite a bit of extra memory .

The patch could use a DenseSet, it just requires a custom DenseMapInfo for ECValue to just hash/compare the data part. I can do that if desired and prepare the patch for review

Compile-time wise this looks neutral, but it would simplify things slightly https://llvm-compile-time-tracker.com/compare.php?from=b0c2ac67a88d3ef86987e2f82115ea0170675a17&to=5c13864badfd7170c9c7c0a908e3cba8aa0f4fea&stat=instructions:u

(the patch just has the unit tests for EQClass removed, because one of the keys doesn't have DenseMapInfo).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shared it here #134264, not sure if it is a huge improvement though


public:
EquivalenceClasses() = default;
EquivalenceClasses(const EquivalenceClasses &RHS) {
Expand All @@ -146,9 +150,10 @@ class EquivalenceClasses {

EquivalenceClasses &operator=(const EquivalenceClasses &RHS) {
TheMapping.clear();
Members.clear();
for (const auto &E : RHS)
if (E.isLeader()) {
member_iterator MI = RHS.member_begin(E);
if (E->isLeader()) {
member_iterator MI = RHS.member_begin(*E);
member_iterator LeaderIt = member_begin(insert(*MI));
for (++MI; MI != member_end(); ++MI)
unionSets(LeaderIt, member_begin(insert(*MI)));
Expand All @@ -161,19 +166,18 @@ class EquivalenceClasses {
//

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

iterator begin() const { return TheMapping.begin(); }
iterator end() const { return TheMapping.end(); }
iterator begin() const { return Members.begin(); }
iterator end() const { return Members.end(); }

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

/// member_* Iterate over the members of an equivalence class.
class member_iterator;
member_iterator member_begin(const ECValue &ECV) const {
// Only leaders provide anything to iterate over.
return member_iterator(ECV.isLeader() ? ECV.getLeader() : nullptr);
return member_iterator(ECV.isLeader() ? &ECV : nullptr);
}

member_iterator member_end() const {
Expand Down Expand Up @@ -208,7 +212,7 @@ class EquivalenceClasses {
unsigned getNumClasses() const {
unsigned NC = 0;
for (const auto &E : *this)
if (E.isLeader())
if (E->isLeader())
++NC;
return NC;
}
Expand All @@ -219,7 +223,10 @@ class EquivalenceClasses {
/// insert - Insert a new value into the union/find set, ignoring the request
/// if the value already exists.
const ECValue &insert(const ElemTy &Data) {
return *TheMapping.insert(ECValue(Data)).first;
auto I = TheMapping.insert(ECValue(Data));
if (I.second)
Members.push_back(&*I.first);
return *I.first;
}

/// findLeader - Given a value in the set, return a member iterator for the
Expand Down
8 changes: 4 additions & 4 deletions llvm/lib/Analysis/VectorUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,10 +844,10 @@ llvm::computeMinimumValueSizes(ArrayRef<BasicBlock *> Blocks, DemandedBits &DB,
DBits[ECs.getOrInsertLeaderValue(I.first)] |= ~0ULL;

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

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

for (Value *M : make_range(ECs.member_begin(E), ECs.member_end())) {
for (Value *M : make_range(ECs.member_begin(*E), ECs.member_end())) {
auto *MI = dyn_cast<Instruction>(M);
if (!MI)
continue;
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/AArch64/AArch64A57FPLoadBalancing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,9 @@ bool AArch64A57FPLoadBalancing::runOnBasicBlock(MachineBasicBlock &MBB) {
// Convert the EquivalenceClasses to a simpler set of sets.
std::vector<std::vector<Chain*> > V;
for (const auto &E : EC) {
if (!E.isLeader())
if (!E->isLeader())
continue;
std::vector<Chain *> Cs(EC.member_begin(E), EC.member_end());
std::vector<Chain *> Cs(EC.member_begin(*E), EC.member_end());
if (Cs.empty()) continue;
V.push_back(std::move(Cs));
}
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/AMDGPU/AMDGPUSplitModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,11 +1017,12 @@ void RecursiveSearchSplitting::setupWorkList() {
}

for (const auto &Node : NodeEC) {
if (!Node.isLeader())
if (!Node->isLeader())
continue;

BitVector Cluster = SG.createNodesBitVector();
for (auto MI = NodeEC.member_begin(Node); MI != NodeEC.member_end(); ++MI) {
for (auto MI = NodeEC.member_begin(*Node); MI != NodeEC.member_end();
++MI) {
const SplitGraph::Node &N = SG.getNode(*MI);
if (N.isGraphEntryPoint())
N.getDependencies(Cluster);
Expand Down
29 changes: 5 additions & 24 deletions llvm/lib/Transforms/IPO/LowerTypeTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2339,36 +2339,17 @@ bool LowerTypeTestsModule::lower() {
if (GlobalClasses.empty())
return false;

// Build a list of disjoint sets ordered by their maximum global index for
// determinism.
std::vector<std::pair<GlobalClassesTy::iterator, unsigned>> Sets;
for (GlobalClassesTy::iterator I = GlobalClasses.begin(),
E = GlobalClasses.end();
I != E; ++I) {
if (!I->isLeader())
// For each disjoint set we found...
for (const auto &C : GlobalClasses) {
if (!C->isLeader())
continue;
++NumTypeIdDisjointSets;

unsigned MaxUniqueId = 0;
for (GlobalClassesTy::member_iterator MI = GlobalClasses.member_begin(*I);
MI != GlobalClasses.member_end(); ++MI) {
if (auto *MD = dyn_cast_if_present<Metadata *>(*MI))
MaxUniqueId = std::max(MaxUniqueId, TypeIdInfo[MD].UniqueId);
else if (auto *BF = dyn_cast_if_present<ICallBranchFunnel *>(*MI))
MaxUniqueId = std::max(MaxUniqueId, BF->UniqueId);
}
Sets.emplace_back(I, MaxUniqueId);
}
llvm::sort(Sets, llvm::less_second());

// For each disjoint set we found...
for (const auto &S : Sets) {
++NumTypeIdDisjointSets;
// Build the list of type identifiers in this disjoint set.
std::vector<Metadata *> TypeIds;
std::vector<GlobalTypeMember *> Globals;
std::vector<ICallBranchFunnel *> ICallBranchFunnels;
for (GlobalClassesTy::member_iterator MI =
GlobalClasses.member_begin(*S.first);
for (GlobalClassesTy::member_iterator MI = GlobalClasses.member_begin(*C);
MI != GlobalClasses.member_end(); ++MI) {
if (isa<Metadata *>(*MI))
TypeIds.push_back(cast<Metadata *>(*MI));
Expand Down
10 changes: 6 additions & 4 deletions llvm/lib/Transforms/Scalar/Float2Int.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,16 @@ bool Float2IntPass::validateAndTransform(const DataLayout &DL) {

// Iterate over every disjoint partition of the def-use graph.
for (const auto &E : ECs) {
if (!E.isLeader())
if (!E->isLeader())
continue;

ConstantRange R(MaxIntegerBW + 1, false);
bool Fail = false;
Type *ConvertedToTy = nullptr;

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

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

for (auto MI = ECs.member_begin(E), ME = ECs.member_end(); MI != ME; ++MI)
for (auto MI = ECs.member_begin(*E), ME = ECs.member_end(); MI != ME; ++MI)
convert(*MI, Ty);
MadeChange = true;
}
Expand Down
6 changes: 3 additions & 3 deletions llvm/lib/Transforms/Scalar/LoopDistribute.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,11 @@ class InstPartitionContainer {
// Merge the member of an equivalence class into its class leader. This
// makes the members empty.
for (const auto &C : ToBeMerged) {
if (!C.isLeader())
if (!C->isLeader())
continue;

auto PartI = C.getData();
for (auto *PartJ : make_range(std::next(ToBeMerged.member_begin(C)),
auto PartI = C->getData();
for (auto *PartJ : make_range(std::next(ToBeMerged.member_begin(*C)),
ToBeMerged.member_end())) {
PartJ->moveTo(*PartI);
}
Expand Down
32 changes: 8 additions & 24 deletions llvm/lib/Transforms/Utils/SplitModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,40 +170,24 @@ static void findPartitions(Module &M, ClusterIDMapType &ClusterIDMap,
for (unsigned i = 0; i < N; ++i)
BalancingQueue.push(std::make_pair(i, 0));

using SortType = std::pair<unsigned, ClusterMapType::iterator>;

SmallVector<SortType, 64> Sets;
SmallPtrSet<const GlobalValue *, 32> Visited;

// To guarantee determinism, we have to sort SCC according to size.
// When size is the same, use leader's name.
for (ClusterMapType::iterator I = GVtoClusterMap.begin(),
E = GVtoClusterMap.end();
I != E; ++I)
if (I->isLeader())
Sets.push_back(
std::make_pair(std::distance(GVtoClusterMap.member_begin(*I),
GVtoClusterMap.member_end()),
I));

llvm::sort(Sets, [](const SortType &a, const SortType &b) {
if (a.first == b.first)
return a.second->getData()->getName() > b.second->getData()->getName();
else
return a.first > b.first;
});

for (auto &I : Sets) {
for (const auto &C : GVtoClusterMap) {
if (!C->isLeader())
continue;

unsigned CurrentClusterID = BalancingQueue.top().first;
unsigned CurrentClusterSize = BalancingQueue.top().second;
BalancingQueue.pop();

LLVM_DEBUG(dbgs() << "Root[" << CurrentClusterID << "] cluster_size("
<< I.first << ") ----> " << I.second->getData()->getName()
<< "\n");
<< std::distance(GVtoClusterMap.member_begin(*C),
GVtoClusterMap.member_end())
<< ") ----> " << C->getData()->getName() << "\n");

for (ClusterMapType::member_iterator MI =
GVtoClusterMap.findLeader(*I.second);
for (ClusterMapType::member_iterator MI = GVtoClusterMap.findLeader(*C);
MI != GVtoClusterMap.member_end(); ++MI) {
if (!Visited.insert(*MI).second)
continue;
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/WebAssembly/cfi.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ define void @h() !type !0 {
}

; CHECK-LABEL: f:
; CHECK: .indidx 1
; CHECK: .indidx 2
define void @f() !type !0 {
ret void
}

; CHECK-LABEL: g:
; CHECK: .indidx 2
; CHECK: .indidx 1
define void @g() !type !1 {
ret void
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/TableGen/GlobalISelCombinerEmitter/type-inference.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def infer_complex_tempreg: GICombineRule <
// CHECK-NEXT: Groups for __infer_variadic_outs_match_1: [dst, x]
// CHECK-NEXT: Groups for __infer_variadic_outs_apply_0: [tmp, y]
// CHECK-NEXT: Groups for __infer_variadic_outs_apply_1:
// CHECK-NEXT: Final Type Equivalence Classes: [tmp, dst, x, y] [vec]
// CHECK-NEXT: Final Type Equivalence Classes: [vec] [tmp, dst, x, y]
// CHECK-NEXT: INFER: MachineOperand $tmp -> GITypeOf<$dst>
// CHECK-NEXT: Apply patterns for rule infer_variadic_outs after inference:
// CHECK-NEXT: (CodeGenInstructionPattern name:__infer_variadic_outs_apply_0 G_FNEG operands:[<def>GITypeOf<$dst>:$tmp, $y])
Expand Down
18 changes: 9 additions & 9 deletions llvm/test/Transforms/LowerTypeTests/function-disjoint.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@

target datalayout = "e-p:64:64"

; X64: @f = alias void (), ptr @[[JT0:.*]]
; X64: @g = alias void (), ptr @[[JT1:.*]]
; X64: @f = alias void (), ptr @[[JT0:.*]]

; WASM32: private constant [0 x i8] zeroinitializer
@0 = private unnamed_addr constant [2 x ptr] [ptr @f, ptr @g], align 16

; X64: define hidden void @f.cfi()
; WASM32: define void @f() !type !{{[0-9]+}} !wasm.index ![[I0:[0-9]+]]
; WASM32: define void @f() !type !{{[0-9]+}} !wasm.index ![[I1:[0-9]+]]
define void @f() !type !0 {
ret void
}

; X64: define hidden void @g.cfi()
; WASM32: define void @g() !type !{{[0-9]+}} !wasm.index ![[I1:[0-9]+]]
; WASM32: define void @g() !type !{{[0-9]+}} !wasm.index ![[I0:[0-9]+]]
define void @g() !type !1 {
ret void
}
Expand All @@ -30,20 +30,20 @@ declare i1 @llvm.type.test(ptr %ptr, metadata %bitset) nounwind readnone

define i1 @foo(ptr %p) {
; X64: icmp eq i64 {{.*}}, ptrtoint (ptr @[[JT0]] to i64)
; WASM32: icmp eq i64 {{.*}}, ptrtoint (ptr getelementptr (i8, ptr null, i64 1) to i64)
; WASM32: icmp eq i64 {{.*}}, ptrtoint (ptr getelementptr (i8, ptr null, i64 2) to i64)
%x = call i1 @llvm.type.test(ptr %p, metadata !"typeid1")
; X64: icmp eq i64 {{.*}}, ptrtoint (ptr @[[JT1]] to i64)
; WASM32: icmp eq i64 {{.*}}, ptrtoint (ptr getelementptr (i8, ptr null, i64 2) to i64)
; WASM32: icmp eq i64 {{.*}}, ptrtoint (ptr getelementptr (i8, ptr null, i64 1) to i64)
%y = call i1 @llvm.type.test(ptr %p, metadata !"typeid2")
%z = add i1 %x, %y
ret i1 %z
}

; X64: define private void @[[JT0]]() #{{.*}} align 8 {
; X64: call void asm sideeffect "jmp ${0:c}@plt\0Aint3\0Aint3\0Aint3\0A", "s"(ptr @f.cfi)

; X64: define private void @[[JT1]]() #{{.*}} align 8 {
; X64: call void asm sideeffect "jmp ${0:c}@plt\0Aint3\0Aint3\0Aint3\0A", "s"(ptr @g.cfi)

; WASM32: ![[I0]] = !{i64 1}
; X64: define private void @[[JT0]]() #{{.*}} align 8 {
; X64: call void asm sideeffect "jmp ${0:c}@plt\0Aint3\0Aint3\0Aint3\0A", "s"(ptr @f.cfi)

; WASM32: ![[I1]] = !{i64 2}
; WASM32: ![[I0]] = !{i64 1}
2 changes: 1 addition & 1 deletion llvm/test/Transforms/LowerTypeTests/nonstring.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

target datalayout = "e-p:32:32"

; CHECK: @[[ANAME:.*]] = private constant { i32 }
; CHECK: @[[BNAME:.*]] = private constant { [2 x i32] }
; CHECK: @[[ANAME:.*]] = private constant { i32 }

@a = constant i32 1, !type !0
@b = constant [2 x i32] [i32 2, i32 3], !type !1
Expand Down
Loading