Skip to content

Commit 316da9e

Browse files
Kenoquinnj
authored andcommitted
Don't color roots that don't need it
Also avoid numbering arguments early in the pipeline. Improves performance on small test cases without safepoints.
1 parent 1f27451 commit 316da9e

File tree

2 files changed

+30
-5
lines changed

2 files changed

+30
-5
lines changed

src/llvm-late-gc-lowering.cpp

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,8 @@ int LateLowerGCFrame::Number(State &S, Value *V) {
445445
if (it != S.AllPtrNumbering.end())
446446
return it->second;
447447
int Number;
448-
if (isa<Constant>(CurrentV) ||
449-
((isa<Argument>(CurrentV) || isa<AllocaInst>(CurrentV) ||
450-
isa<AddrSpaceCastInst>(CurrentV)) &&
448+
if (isa<Constant>(CurrentV) || isa<Argument>(CurrentV) ||
449+
((isa<AllocaInst>(CurrentV) || isa<AddrSpaceCastInst>(CurrentV)) &&
451450
getValueAddrSpace(CurrentV) != AddressSpace::Tracked)) {
452451
// We know this is rooted in the parent
453452
Number = -1;
@@ -831,8 +830,9 @@ void LateLowerGCFrame::ComputeLiveSets(Function &F, State &S) {
831830
if ((unsigned)i >= LS.size() || !LS[i])
832831
continue;
833832
for (int Idx = LS.find_first(); Idx >= 0; Idx = LS.find_next(Idx)) {
834-
if (Idx == i)
835-
continue;
833+
// We explicitly let i be a neighbor of itself, to distinguish
834+
// between being the only value live at a safepoint, vs not
835+
// being live at any safepoint.
836836
Neighbors.push_back(Idx);
837837
}
838838
}
@@ -916,6 +916,8 @@ struct PEOIterator {
916916
Elements[NextElement].weight = (unsigned)-1;
917917
// Raise neighbors
918918
for (int Neighbor : Neighbors[NextElement]) {
919+
if (Neighbor == NextElement)
920+
continue;
919921
Element &NElement = Elements[Neighbor];
920922
// Already processed. Don't re-enqueue
921923
if (NElement.weight == (unsigned)-1)
@@ -947,6 +949,10 @@ std::vector<int> LateLowerGCFrame::ColorRoots(const State &S) {
947949
assert(Colors[ActiveElement] == -1);
948950
UsedColors.resize(MaxAssignedColor + 2, false);
949951
UsedColors.reset();
952+
if (S.Neighbors[ActiveElement].empty()) {
953+
// No need to color a value not live at any safe point
954+
continue;
955+
}
950956
for (int Neighbor : S.Neighbors[ActiveElement]) {
951957
if (Colors[Neighbor] == -1)
952958
continue;

test/llvmpasses/gcroots.ll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,22 @@ define %jl_value_t addrspace(10)* @ret_use(i64 %a, i64 %b) {
155155
%bboxed = call %jl_value_t addrspace(10)* @jl_box_int64(i64 signext %b)
156156
ret %jl_value_t addrspace(10)* %aboxed
157157
}
158+
159+
define i8 @nosafepoint(%jl_value_t addrspace(10)* dereferenceable(16)) {
160+
; CHECK-LABEL: @nosafepoint
161+
; CHECK-NOT: %gcframe
162+
top:
163+
%1 = call %jl_value_t*** @jl_get_ptls_states()
164+
%2 = bitcast %jl_value_t*** %1 to %jl_value_t addrspace(10)**
165+
%3 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %2, i64 3
166+
%4 = bitcast %jl_value_t addrspace(10)** %3 to i64**
167+
%5 = load i64*, i64** %4
168+
%6 = bitcast %jl_value_t addrspace(10)* %0 to i8 addrspace(10)*
169+
%7 = addrspacecast i8 addrspace(10)* %6 to i8 addrspace(11)*
170+
%8 = getelementptr i8, i8 addrspace(11)* %7, i64 0
171+
%9 = load i8, i8 addrspace(11)* %8
172+
%10 = trunc i8 %9 to i1
173+
%11 = zext i1 %10 to i8
174+
%12 = xor i8 %11, 1
175+
ret i8 %12
176+
}

0 commit comments

Comments
 (0)