Skip to content
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.

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

Differentiate OBJ_PIN and PTR_PIN. Add more pinning. Trace all global roots. #84

Merged
merged 5 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/aotcompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ typedef struct {
SmallVector<GlobalValue*, 0> jl_sysimg_fvars;
SmallVector<GlobalValue*, 0> jl_sysimg_gvars;
std::map<jl_code_instance_t*, std::tuple<uint32_t, uint32_t>> jl_fvar_map;
// This holds references to the heap. Need to be pinned.
SmallVector<void*, 0> jl_value_to_llvm;
SmallVector<jl_code_instance_t*, 0> jl_external_to_llvm;
} jl_native_code_desc_t;
Expand Down
4 changes: 3 additions & 1 deletion src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,9 @@ static value_t julia_to_list2_noalloc(fl_context_t *fl_ctx, jl_value_t *a, jl_va

static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v, int check_valid)
{
PTR_PIN(v);
// The following code will take internal pointers to v's fields. We need to make sure
// that v will not be moved by GC.
OBJ_PIN(v);
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
value_t retval;
if (julia_to_scm_noalloc1(fl_ctx, v, &retval))
return retval;
Expand Down
6 changes: 3 additions & 3 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
}
// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
PTR_PIN(v);
OBJ_PIN(v);
uintptr_t bits = jl_astaggedvalue(v)->header;
if (bits & GC_IN_IMAGE)
return ((uintptr_t*)v)[-2];
Expand Down Expand Up @@ -406,7 +406,7 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT

// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
PTR_PIN(v);
PTR_PIN(v); // This has to be a pointer pin -- v could be an internal pointer
return bits_hash(v, sz) ^ h;
}
if (dt == jl_unionall_type)
Expand Down Expand Up @@ -470,7 +470,7 @@ static uintptr_t NOINLINE jl_object_id__cold(uintptr_t tv, jl_value_t *v) JL_NOT

// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
PTR_PIN(v);
OBJ_PIN(v);
return inthash((uintptr_t)v);
}
return immut_id_(dt, v, dt->hash);
Expand Down
4 changes: 3 additions & 1 deletion src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ static Constant *julia_pgv(jl_codectx_t &ctx, const char *cname, void *addr)
// emit a GlobalVariable for a jl_value_t named "cname"
// store the name given so we can reuse it (facilitating merging later)
// so first see if there already is a GlobalVariable for this address
OBJ_PIN(addr); // This will be stored in the native heap. We need to pin it.
GlobalVariable* &gv = ctx.emission_context.global_targets[addr];
Module *M = jl_Module;
StringRef localname;
Expand Down Expand Up @@ -570,7 +571,8 @@ static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p)
{
if (p == NULL)
return Constant::getNullValue(ctx.types().T_pjlvalue);
PTR_PIN(p);
// Pointers to p will be emitted into the code. Make sure p won't be moved by GC.
OBJ_PIN(p);
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
Value *pgv = literal_pointer_val_slot(ctx, p);
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_const);
auto load = ai.decorateInst(maybe_mark_load_dereferenceable(
Expand Down
10 changes: 10 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1823,6 +1823,7 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
assert(TIndex == nullptr || TIndex->getType() == getInt8Ty(TIndex->getContext()));
}
jl_cgval_t(Value *Vptr, bool isboxed, jl_value_t *typ, Value *tindex, MDNode *tbaa, Value* inline_roots) = delete;
Expand All @@ -1839,6 +1840,7 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
if (Vboxed)
assert(Vboxed->getType() == JuliaType::get_prjlvalue_ty(Vboxed->getContext()));
assert(tbaa != nullptr);
Expand All @@ -1859,6 +1861,8 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
OBJ_PIN(constant); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
assert(jl_is_datatype(typ));
assert(constant);
}
Expand All @@ -1875,6 +1879,8 @@ struct jl_cgval_t {
promotion_point(v.promotion_point),
promotion_ssa(v.promotion_ssa)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
OBJ_PIN(constant); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
if (Vboxed)
assert(Vboxed->getType() == JuliaType::get_prjlvalue_ty(Vboxed->getContext()));
// this constructor expects we had a badly or equivalently typed version
Expand Down Expand Up @@ -1947,6 +1953,7 @@ class jl_codectx_t {
std::map<int, jl_varinfo_t> phic_slots;
std::map<int, std::pair<Value*, Value*> > scope_restore;
SmallVector<jl_cgval_t, 0> SAvalues;
// The vector holds reference to Julia obj ref. We need to pin jl_value_t*.
SmallVector<std::tuple<jl_cgval_t, BasicBlock *, AllocaInst *, PHINode *, SmallVector<PHINode*,0>, jl_value_t *>, 0> PhiNodes;
SmallVector<bool, 0> ssavalue_assigned;
SmallVector<int, 0> ssavalue_usecount;
Expand Down Expand Up @@ -6254,6 +6261,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
decay_derived(ctx, phi));
jl_cgval_t val = mark_julia_slot(ptr, phiType, Tindex_phi, best_tbaa(ctx.tbaa(), phiType));
val.Vboxed = ptr_phi;
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(val, BB, dest, ptr_phi, roots, r));
ctx.SAvalues[idx] = val;
ctx.ssavalue_assigned[idx] = true;
Expand All @@ -6263,6 +6271,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
PHINode *Tindex_phi = PHINode::Create(getInt8Ty(ctx.builder.getContext()), jl_array_nrows(edges), "tindex_phi");
Tindex_phi->insertInto(BB, InsertPt);
jl_cgval_t val = mark_julia_slot(NULL, phiType, Tindex_phi, ctx.tbaa().tbaa_stack);
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(val, BB, dest, (PHINode*)nullptr, roots, r));
ctx.SAvalues[idx] = val;
ctx.ssavalue_assigned[idx] = true;
Expand Down Expand Up @@ -6313,6 +6322,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
value_phi->insertInto(BB, InsertPt);
slot = mark_julia_type(ctx, value_phi, isboxed, phiType);
}
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(slot, BB, dest, value_phi, roots, r));
ctx.SAvalues[idx] = slot;
ctx.ssavalue_assigned[idx] = true;
Expand Down
4 changes: 2 additions & 2 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ JL_DLLEXPORT jl_typename_t *jl_new_typename_in(jl_sym_t *name, jl_module_t *modu
jl_typename_type);
// Typenames should be pinned since they are used as metadata, and are
// read during scan_object
PTR_PIN(tn);
OBJ_PIN(tn);
Copy link

Choose a reason for hiding this comment

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

Should typenames be allocated in a non-moving space? Maybe add a FIXME comment saying that instead of pinning them, we might do that instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a commit to add an allocation function jl_gc_alloc_non_moving_ and use it for cases like this. Do you want me to include the change in this PR? I planned to have it as a separate PR after this one, but I can include it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This commit: qinsoon@70a4e14

Copy link

Choose a reason for hiding this comment

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

Happy for it to be a separate PR. 👍

tn->name = name;
tn->module = module;
tn->wrapper = NULL;
Expand Down Expand Up @@ -101,7 +101,7 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
jl_datatype_t *t = (jl_datatype_t*)jl_gc_alloc(ct->ptls, sizeof(jl_datatype_t), jl_datatype_type);
// Types should be pinned since they are used as metadata, and are
// read during scan_object
PTR_PIN(t);
OBJ_PIN(t);
qinsoon marked this conversation as resolved.
Show resolved Hide resolved
jl_set_typetagof(t, jl_datatype_tag, 0);
t->hash = 0;
t->hasfreetypevars = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/gc-interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection);
JL_DLLEXPORT int gc_is_collector_thread(int tid) JL_NOTSAFEPOINT;
// Pinning objects; Returns whether the object has been pinned by this call.
JL_DLLEXPORT unsigned char jl_gc_pin_object(void* obj);
// Pinning objects through a potential internal pointer; Returns whether the object has been pinned by this call.
JL_DLLEXPORT unsigned char jl_gc_pin_pointer(void* ptr);
// Returns which GC implementation is being used and possibly its version according to the list of supported GCs
// NB: it should clearly identify the GC by including e.g. ‘stock’ or ‘mmtk’ as a substring.
JL_DLLEXPORT const char* jl_gc_active_impl(void);
Expand Down
Loading