Skip to content

Commit

Permalink
[late-gc-lowering] null-out GC frame slots for dead objects (#52935)
Browse files Browse the repository at this point in the history 
Should fix #51818.

MWE:

```julia
function testme()
     X = @noinline rand(1_000_000_00)
     Y = @noinline sum(X)
     X = nothing
     GC.gc()
     return Y
 end
```

Note that it now stores a `NULL` in the GC frame before calling
`jl_gc_collect`.

Before:

```llvm
; Function Signature: testme()
;  @ /Users/dnetto/Personal/test.jl:3 within `testme`
define double @julia_testme_535() #0 {
top:
  %gcframe1 = alloca [3 x ptr], align 16
  call void @llvm.memset.p0.i64(ptr align 16 %gcframe1, i8 0, i64 24, i1 true)
  %pgcstack = call ptr inttoptr (i64 6595051180 to ptr)(i64 262) #10
  store i64 4, ptr %gcframe1, align 16
  %task.gcstack = load ptr, ptr %pgcstack, align 8
  %frame.prev = getelementptr inbounds ptr, ptr %gcframe1, i64 1
  store ptr %task.gcstack, ptr %frame.prev, align 8
  store ptr %gcframe1, ptr %pgcstack, align 8
;  @ /Users/dnetto/Personal/test.jl:4 within `testme`
  %0 = call nonnull ptr @j_rand_539(i64 signext 100000000)
  %gc_slot_addr_0 = getelementptr inbounds ptr, ptr %gcframe1, i64 2
  store ptr %0, ptr %gc_slot_addr_0, align 16
;  @ /Users/dnetto/Personal/test.jl:5 within `testme`
  %1 = call double @j_sum_541(ptr nonnull %0)
;  @ /Users/dnetto/Personal/test.jl:7 within `testme`
; ┌ @ gcutils.jl:132 within `gc` @ gcutils.jl:132
   call void @jlplt_ijl_gc_collect_543_got.jit(i32 1)
   %frame.prev4 = load ptr, ptr %frame.prev, align 8
   store ptr %frame.prev4, ptr %pgcstack, align 8
; └
;  @ /Users/dnetto/Personal/test.jl:8 within `testme`
  ret double %1
}
```

After:

```llvm
; Function Signature: testme()
;  @ /Users/dnetto/Personal/test.jl:3 within `testme`
define double @julia_testme_752() #0 {
top:
  %gcframe1 = alloca [3 x ptr], align 16
  call void @llvm.memset.p0.i64(ptr align 16 %gcframe1, i8 0, i64 24, i1 true)
  %pgcstack = call ptr inttoptr (i64 6595051180 to ptr)(i64 262) #10
  store i64 4, ptr %gcframe1, align 16
  %task.gcstack = load ptr, ptr %pgcstack, align 8
  %frame.prev = getelementptr inbounds ptr, ptr %gcframe1, i64 1
  store ptr %task.gcstack, ptr %frame.prev, align 8
  store ptr %gcframe1, ptr %pgcstack, align 8
;  @ /Users/dnetto/Personal/test.jl:4 within `testme`
  %0 = call nonnull ptr @j_rand_756(i64 signext 100000000)
  %gc_slot_addr_0 = getelementptr inbounds ptr, ptr %gcframe1, i64 2
  store ptr %0, ptr %gc_slot_addr_0, align 16
;  @ /Users/dnetto/Personal/test.jl:5 within `testme`
  %1 = call double @j_sum_758(ptr nonnull %0)
  store ptr null, ptr %gc_slot_addr_0, align 16
;  @ /Users/dnetto/Personal/test.jl:7 within `testme`
; ┌ @ gcutils.jl:132 within `gc` @ gcutils.jl:132
   call void @jlplt_ijl_gc_collect_760_got.jit(i32 1)
   %frame.prev6 = load ptr, ptr %frame.prev, align 8
   store ptr %frame.prev6, ptr %pgcstack, align 8
; └
;  @ /Users/dnetto/Personal/test.jl:8 within `testme`
  ret double %1
}
```
  • Loading branch information
@d-netto
d-netto authored Nov 6, 2024
1 parent bce3d4d commit efc43cb
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 12 deletions.
7 changes: 4 additions & 3 deletions src/llvm-gc-interface-passes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,11 @@ struct LateLowerGCFrame: private JuliaPassContext {
State LocalScan(Function &F);
void ComputeLiveness(State &S);
void ComputeLiveSets(State &S);
SmallVector<int, 0> ColorRoots(const State &S);
std::pair<SmallVector<int, 0>, int> ColorRoots(const State &S);
void PlaceGCFrameStore(State &S, unsigned R, unsigned MinColorRoot, ArrayRef<int> Colors, Value *GCFrame, Instruction *InsertBefore);
void PlaceGCFrameStores(State &S, unsigned MinColorRoot, ArrayRef<int> Colors, Value *GCFrame);
void PlaceRootsAndUpdateCalls(SmallVectorImpl<int> &Colors, State &S, std::map<Value *, std::pair<int, int>>);
void PlaceGCFrameStores(State &S, unsigned MinColorRoot, ArrayRef<int> Colors, int PreAssignedColors, Value *GCFrame);
void PlaceGCFrameReset(State &S, unsigned R, unsigned MinColorRoot, ArrayRef<int> Colors, Value *GCFrame, Instruction *InsertBefore);
void PlaceRootsAndUpdateCalls(ArrayRef<int> Colors, int PreAssignedColors, State &S, std::map<Value *, std::pair<int, int>>);
void CleanupWriteBarriers(Function &F, State *S, const SmallVector<CallInst*, 0> &WriteBarriers, bool *CFGModified);
bool CleanupIR(Function &F, State *S, bool *CFGModified);
void NoteUseChain(State &S, BBState &BBS, User *TheUser);
Expand Down
36 changes: 29 additions & 7 deletions src/llvm-late-gc-lowering.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1820,7 +1820,7 @@ JL_USED_FUNC static void dumpColorAssignments(const State &S, const ArrayRef<int
}
}

SmallVector<int, 0> LateLowerGCFrame::ColorRoots(const State &S) {
std::pair<SmallVector<int, 0>, int> LateLowerGCFrame::ColorRoots(const State &S) {
SmallVector<int, 0> Colors;
Colors.resize(S.MaxPtrNumber + 1, -1);
PEOIterator Ordering(S.Neighbors);
Expand Down Expand Up @@ -1862,7 +1862,7 @@ SmallVector<int, 0> LateLowerGCFrame::ColorRoots(const State &S) {
NewColor += PreAssignedColors;
Colors[ActiveElement] = NewColor;
}
return Colors;
return {Colors, PreAssignedColors};
}

// Size of T is assumed to be `sizeof(void*)`
Expand Down Expand Up @@ -2292,8 +2292,21 @@ void LateLowerGCFrame::PlaceGCFrameStore(State &S, unsigned R, unsigned MinColor
new StoreInst(Val, slotAddress, InsertBefore);
}

void LateLowerGCFrame::PlaceGCFrameReset(State &S, unsigned R, unsigned MinColorRoot,
ArrayRef<int> Colors, Value *GCFrame,
Instruction *InsertBefore) {
// Get the slot address.
auto slotAddress = CallInst::Create(
getOrDeclare(jl_intrinsics::getGCFrameSlot),
{GCFrame, ConstantInt::get(Type::getInt32Ty(InsertBefore->getContext()), Colors[R] + MinColorRoot)},
"gc_slot_addr_" + StringRef(std::to_string(Colors[R] + MinColorRoot)), InsertBefore);
// Reset the slot to NULL.
Value *Val = ConstantPointerNull::get(T_prjlvalue);
new StoreInst(Val, slotAddress, InsertBefore);
}

void LateLowerGCFrame::PlaceGCFrameStores(State &S, unsigned MinColorRoot,
ArrayRef<int> Colors, Value *GCFrame)
ArrayRef<int> Colors, int PreAssignedColors, Value *GCFrame)
{
for (auto &BB : *S.F) {
const BBState &BBS = S.BBStates[&BB];
Expand All @@ -2306,6 +2319,15 @@ void LateLowerGCFrame::PlaceGCFrameStores(State &S, unsigned MinColorRoot,
for(auto rit = BBS.Safepoints.rbegin();
rit != BBS.Safepoints.rend(); ++rit ) {
const LargeSparseBitVector &NowLive = S.LiveSets[*rit];
// reset slots which are no longer alive
for (int Idx : *LastLive) {
if (Idx >= PreAssignedColors && !HasBitSet(NowLive, Idx)) {
PlaceGCFrameReset(S, Idx, MinColorRoot, Colors, GCFrame,
S.ReverseSafepointNumbering[*rit]);
}
}
// store values which are alive in this safepoint but
// haven't been stored in the GC frame before
for (int Idx : NowLive) {
if (!HasBitSet(*LastLive, Idx)) {
PlaceGCFrameStore(S, Idx, MinColorRoot, Colors, GCFrame,
Expand All @@ -2317,7 +2339,7 @@ void LateLowerGCFrame::PlaceGCFrameStores(State &S, unsigned MinColorRoot,
}
}

void LateLowerGCFrame::PlaceRootsAndUpdateCalls(SmallVectorImpl<int> &Colors, State &S,
void LateLowerGCFrame::PlaceRootsAndUpdateCalls(ArrayRef<int> Colors, int PreAssignedColors, State &S,
std::map<Value *, std::pair<int, int>>) {
auto F = S.F;
auto T_int32 = Type::getInt32Ty(F->getContext());
Expand Down Expand Up @@ -2439,7 +2461,7 @@ void LateLowerGCFrame::PlaceRootsAndUpdateCalls(SmallVectorImpl<int> &Colors, St
pushGcframe->setArgOperand(1, NRoots);

// Insert GC frame stores
PlaceGCFrameStores(S, AllocaSlot - 2, Colors, gcframe);
PlaceGCFrameStores(S, AllocaSlot - 2, Colors, PreAssignedColors, gcframe);
// Insert GCFrame pops
for (auto &BB : *F) {
if (isa<ReturnInst>(BB.getTerminator())) {
Expand All @@ -2464,9 +2486,9 @@ bool LateLowerGCFrame::runOnFunction(Function &F, bool *CFGModified) {

State S = LocalScan(F);
ComputeLiveness(S);
SmallVector<int, 0> Colors = ColorRoots(S);
auto Colors = ColorRoots(S);
std::map<Value *, std::pair<int, int>> CallFrames; // = OptimizeCallFrames(S, Ordering);
PlaceRootsAndUpdateCalls(Colors, S, CallFrames);
PlaceRootsAndUpdateCalls(Colors.first, Colors.second, S, CallFrames);
CleanupIR(F, &S, CFGModified);
return true;
}
Expand Down
22 changes: 22 additions & 0 deletions test/compiler/codegen.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1003,3 +1003,25 @@ end
@test f55768(Vector)
@test f55768(Vector{T} where T)
@test !f55768(Vector{S} where S)

# test that values get rooted correctly over throw
for a in ((@noinline Ref{Int}(2)),
(@noinline Ref{Int}(3)),
5,
(@noinline Ref{Int}(4)),
6)
@test a[] != 0
try
b = (@noinline Ref{Int}(5),
@noinline Ref{Int}(6),
@noinline Ref{Int}(7),
@noinline Ref{Int}(8),
@noinline Ref{Int}(9),
@noinline Ref{Int}(10),
@noinline Ref{Int}(11))
GC.gc(true)
GC.@preserve b throw(a)
catch ex
@test ex === a
end
end
9 changes: 7 additions & 2 deletions test/llvmpasses/returnstwicegc.ll
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
; This file is a part of Julia. License is MIT: https://julialang.org/license

; RUN: opt --load-pass-plugin=libjulia-codegen%shlibext -passes='function(LateLowerGCFrame,FinalLowerGC)' -S %s | FileCheck %s --check-prefixes=OPAQUE
; RUN: opt --load-pass-plugin=libjulia-codegen%shlibext -passes='function(LateLowerGCFrame,FinalLowerGC)' -S %s | FileCheck %s


declare void @boxed_simple({} addrspace(10)*, {} addrspace(10)*)
Expand All @@ -13,7 +13,12 @@ declare void @one_arg_boxed({} addrspace(10)*)
define void @try_catch(i64 %a, i64 %b)
{
; Because of the returns_twice function, we need to keep aboxed live everywhere
; OPAQUE: %gcframe = alloca ptr addrspace(10), i32 4
; CHECK: %gcframe = alloca ptr addrspace(10), i32 4
; CHECK: store ptr addrspace(10) %aboxed, ptr [[slot_0:%.*]],
; CHECK-NOT: store {{.*}} ptr [[slot_0]]
; CHECK: store ptr addrspace(10) %bboxed, ptr {{%.*}}
; CHECK-NOT: store {{.*}} ptr [[slot_0]]

top:
%sigframe = alloca [208 x i8], align 16
%sigframe.sub = getelementptr inbounds [208 x i8], [208 x i8]* %sigframe, i64 0, i64 0
Expand Down

0 comments on commit efc43cb

Please sign in to comment.