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.
JuliaLang · Jun 14, 2017 · 693747a · 693747a
1 parent 56b942e
commit 693747a
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 5 deletions.
diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp
@@ -445,9 +445,8 @@ int LateLowerGCFrame::Number(State &S, Value *V) {
     if (it != S.AllPtrNumbering.end())
         return it->second;
     int Number;
-    if (isa<Constant>(CurrentV) ||
-        ((isa<Argument>(CurrentV) || isa<AllocaInst>(CurrentV) ||
-         isa<AddrSpaceCastInst>(CurrentV)) &&
+    if (isa<Constant>(CurrentV) || isa<Argument>(CurrentV) ||
+        ((isa<AllocaInst>(CurrentV) || isa<AddrSpaceCastInst>(CurrentV)) &&
          getValueAddrSpace(CurrentV) != AddressSpace::Tracked)) {
         // We know this is rooted in the parent
         Number = -1;
@@ -831,8 +830,9 @@ void LateLowerGCFrame::ComputeLiveSets(Function &F, State &S) {
             if ((unsigned)i >= LS.size() || !LS[i])
                 continue;
             for (int Idx = LS.find_first(); Idx >= 0; Idx = LS.find_next(Idx)) {
-                if (Idx == i)
-                    continue;
+                // We explicitly let i be a neighbor of itself, to distinguish
+                // between being the only value live at a safepoint, vs not
+                // being live at any safepoint.
                 Neighbors.push_back(Idx);
             }
         }
@@ -916,6 +916,8 @@ struct PEOIterator {
         Elements[NextElement].weight = (unsigned)-1;
         // Raise neighbors
         for (int Neighbor : Neighbors[NextElement]) {
+            if (Neighbor == NextElement)
+                continue;
             Element &NElement = Elements[Neighbor];
             // Already processed. Don't re-enqueue
             if (NElement.weight == (unsigned)-1)
@@ -947,6 +949,10 @@ std::vector<int> LateLowerGCFrame::ColorRoots(const State &S) {
         assert(Colors[ActiveElement] == -1);
         UsedColors.resize(MaxAssignedColor + 2, false);
         UsedColors.reset();
+        if (S.Neighbors[ActiveElement].empty()) {
+            // No need to color a value not live at any safe point
+            continue;
+        }
         for (int Neighbor : S.Neighbors[ActiveElement]) {
             if (Colors[Neighbor] == -1)
                 continue;

diff --git a/test/llvmpasses/gcroots.ll b/test/llvmpasses/gcroots.ll
@@ -155,3 +155,22 @@ define %jl_value_t addrspace(10)* @ret_use(i64 %a, i64 %b) {
     %bboxed = call %jl_value_t addrspace(10)* @jl_box_int64(i64 signext %b)
     ret %jl_value_t addrspace(10)* %aboxed
 }
+
+define i8 @nosafepoint(%jl_value_t addrspace(10)* dereferenceable(16)) {
+; CHECK-LABEL: @nosafepoint
+; CHECK-NOT: %gcframe
+top:
+  %1 = call %jl_value_t*** @jl_get_ptls_states()
+  %2 = bitcast %jl_value_t*** %1 to %jl_value_t addrspace(10)**
+  %3 = getelementptr %jl_value_t addrspace(10)*, %jl_value_t addrspace(10)** %2, i64 3
+  %4 = bitcast %jl_value_t addrspace(10)** %3 to i64**
+  %5 = load i64*, i64** %4
+  %6 = bitcast %jl_value_t addrspace(10)* %0 to i8 addrspace(10)*
+  %7 = addrspacecast i8 addrspace(10)* %6 to i8 addrspace(11)*
+  %8 = getelementptr i8, i8 addrspace(11)* %7, i64 0
+  %9 = load i8, i8 addrspace(11)* %8
+  %10 = trunc i8 %9 to i1
+  %11 = zext i1 %10 to i8
+  %12 = xor i8 %11, 1
+  ret i8 %12
+}