Skip to content

Commit

Permalink
codegen: make gcroots for argument-derived values (JuliaLang#53501)
Browse files Browse the repository at this point in the history
This function might previously refuse to make gc roots for any of the
function's arguments (i.e. it potentially assumed everything was in an
alloca), which was only valid if we could guarantee that no IPO passes
run. This commit aims to make that safe, but deferring such decisions
about their source until llvm-late-gc-lowering can make them valid.

This should make inlining-safe gc annotations for

code_llvm(raw=true, optimize=false, (Some{Int},)) do x; GC.@preserve x
GC.safepoint(); end

and also better annotations even without inlining for

code_llvm(raw=true, optimize=false, (Some{Any},)) do x; GC.@preserve x
GC.safepoint(); end
  • Loading branch information
vtjnash committed Feb 29, 2024
1 parent 24aaf00 commit 715c50d
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 79 deletions.
10 changes: 4 additions & 6 deletions src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ class function_sig_t {
jl_codectx_t &ctx,
const native_sym_arg_t &symarg,
jl_cgval_t *argv,
SmallVector<Value*, 16> &gc_uses,
SmallVectorImpl<Value*> &gc_uses,
bool static_rt) const;

private:
Expand Down Expand Up @@ -1389,16 +1389,14 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
}

// emit roots
SmallVector<Value*, 16> gc_uses;
SmallVector<Value*> gc_uses;
for (size_t i = nccallargs + fc_args_start; i <= nargs; i++) {
// Julia (expression) value of current parameter gcroot
jl_value_t *argi_root = args[i];
if (jl_is_long(argi_root))
continue;
jl_cgval_t arg_root = emit_expr(ctx, argi_root);
Value *gc_root = get_gc_root_for(ctx, arg_root);
if (gc_root)
gc_uses.push_back(gc_root);
gc_uses.append(get_gc_roots_for(ctx, arg_root));
}

jl_unionall_t *unionall = (jl_is_method(ctx.linfo->def.method) && jl_is_unionall(ctx.linfo->def.method->sig))
Expand Down Expand Up @@ -1855,7 +1853,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
jl_codectx_t &ctx,
const native_sym_arg_t &symarg,
jl_cgval_t *argv,
SmallVector<Value*, 16> &gc_uses,
SmallVectorImpl<Value*> &gc_uses,
bool static_rt) const
{
++EmittedCCalls;
Expand Down
104 changes: 51 additions & 53 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,32 +312,64 @@ static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt);
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, unsigned alignment, bool isVolatile=false);

static Value *get_gc_root_for(jl_codectx_t &ctx, const jl_cgval_t &x)
static bool type_is_permalloc(jl_value_t *typ)
{
// Singleton should almost always be handled by the later optimization passes.
// Also do it here since it is cheap and save some effort in LLVM passes.
if (jl_is_datatype(typ) && jl_is_datatype_singleton((jl_datatype_t*)typ))
return true;
return typ == (jl_value_t*)jl_symbol_type ||
typ == (jl_value_t*)jl_int8_type ||
typ == (jl_value_t*)jl_uint8_type;
}


static void find_perm_offsets(jl_datatype_t *typ, SmallVectorImpl<unsigned> &res, unsigned offset)
{
// This is a inlined field at `offset`.
if (!typ->layout || typ->layout->npointers == 0)
return;
jl_svec_t *types = jl_get_fieldtypes(typ);
size_t nf = jl_svec_len(types);
for (size_t i = 0; i < nf; i++) {
jl_value_t *_fld = jl_svecref(types, i);
if (!jl_is_datatype(_fld))
continue;
jl_datatype_t *fld = (jl_datatype_t*)_fld;
if (jl_field_isptr(typ, i)) {
// pointer field, check if field is perm-alloc
if (type_is_permalloc((jl_value_t*)fld))
res.push_back(offset + jl_field_offset(typ, i));
continue;
}
// inline field
find_perm_offsets(fld, res, offset + jl_field_offset(typ, i));
}
}

static llvm::SmallVector<llvm::Value*, 0> get_gc_roots_for(jl_codectx_t &ctx, const jl_cgval_t &x)
{
if (x.constant || x.typ == jl_bottom_type)
return nullptr;
return {};
if (x.Vboxed) // superset of x.isboxed
return x.Vboxed;
return {x.Vboxed};
assert(!x.isboxed);
#ifndef NDEBUG
if (x.ispointer()) {
assert(x.V);
if (PointerType *T = dyn_cast<PointerType>(x.V->getType())) {
assert(T->getAddressSpace() != AddressSpace::Tracked);
if (T->getAddressSpace() == AddressSpace::Derived) {
// n.b. this IR would not be valid after LLVM-level inlining,
// since codegen does not have a way to determine the whether
// this argument value needs to be re-rooted
}
}
}
#endif
if (jl_is_concrete_immutable(x.typ) && !jl_is_pointerfree(x.typ)) {
Type *T = julia_type_to_llvm(ctx, x.typ);
return emit_unbox(ctx, T, x, x.typ);
assert(x.V->getType()->getPointerAddressSpace() != AddressSpace::Tracked);
return {x.V};
}
else if (jl_is_concrete_immutable(x.typ) && !jl_is_pointerfree(x.typ)) {
jl_value_t *jltype = x.typ;
Type *T = julia_type_to_llvm(ctx, jltype);
Value *agg = emit_unbox(ctx, T, x, jltype);
SmallVector<unsigned,4> perm_offsets;
if (jltype && jl_is_datatype(jltype) && ((jl_datatype_t*)jltype)->layout)
find_perm_offsets((jl_datatype_t*)jltype, perm_offsets, 0);
return ExtractTrackedValues(agg, agg->getType(), false, ctx.builder, perm_offsets);
}
// nothing here to root, move along
return nullptr;
return {};
}

// --- emitting pointers directly into code ---
Expand Down Expand Up @@ -590,17 +622,6 @@ static Value *julia_binding_gv(jl_codectx_t &ctx, jl_binding_t *b)

// --- mapping between julia and llvm types ---

static bool type_is_permalloc(jl_value_t *typ)
{
// Singleton should almost always be handled by the later optimization passes.
// Also do it here since it is cheap and save some effort in LLVM passes.
if (jl_is_datatype(typ) && jl_is_datatype_singleton((jl_datatype_t*)typ))
return true;
return typ == (jl_value_t*)jl_symbol_type ||
typ == (jl_value_t*)jl_int8_type ||
typ == (jl_value_t*)jl_uint8_type;
}

static unsigned convert_struct_offset(const llvm::DataLayout &DL, Type *lty, unsigned byte_offset)
{
const StructLayout *SL = DL.getStructLayout(cast<StructType>(lty));
Expand Down Expand Up @@ -1905,7 +1926,7 @@ Value *extract_first_ptr(jl_codectx_t &ctx, Value *V)
if (path.empty())
return NULL;
std::reverse(std::begin(path), std::end(path));
return ctx.builder.CreateExtractValue(V, path);
return CreateSimplifiedExtractValue(ctx, V, path);
}


Expand Down Expand Up @@ -3590,29 +3611,6 @@ static void emit_write_barrier(jl_codectx_t &ctx, Value *parent, ArrayRef<Value*
ctx.builder.CreateCall(prepare_call(jl_write_barrier_func), decay_ptrs);
}

static void find_perm_offsets(jl_datatype_t *typ, SmallVectorImpl<unsigned> &res, unsigned offset)
{
// This is a inlined field at `offset`.
if (!typ->layout || typ->layout->npointers == 0)
return;
jl_svec_t *types = jl_get_fieldtypes(typ);
size_t nf = jl_svec_len(types);
for (size_t i = 0; i < nf; i++) {
jl_value_t *_fld = jl_svecref(types, i);
if (!jl_is_datatype(_fld))
continue;
jl_datatype_t *fld = (jl_datatype_t*)_fld;
if (jl_field_isptr(typ, i)) {
// pointer field, check if field is perm-alloc
if (type_is_permalloc((jl_value_t*)fld))
res.push_back(offset + jl_field_offset(typ, i));
continue;
}
// inline field
find_perm_offsets(fld, res, offset + jl_field_offset(typ, i));
}
}

static void emit_write_multibarrier(jl_codectx_t &ctx, Value *parent, Value *agg,
jl_value_t *jltype)
{
Expand Down
17 changes: 6 additions & 11 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3365,21 +3365,18 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t a
value_to_pointer(ctx, arg2).V;
varg1 = emit_pointer_from_objref(ctx, varg1);
varg2 = emit_pointer_from_objref(ctx, varg2);
Value *gc_uses[2];
int nroots = 0;
SmallVector<Value*, 0> gc_uses;
// these roots may seem a bit overkill, but we want to make sure
// that a!=b implies (a,)!=(b,) even if a and b are unused and
// therefore could be freed and then the memory for a reused for b
if ((gc_uses[nroots] = get_gc_root_for(ctx, arg1)))
nroots++;
if ((gc_uses[nroots] = get_gc_root_for(ctx, arg2)))
nroots++;
OperandBundleDef OpBundle("jl_roots", ArrayRef<Value*>(gc_uses, nroots));
gc_uses.append(get_gc_roots_for(ctx, arg1));
gc_uses.append(get_gc_roots_for(ctx, arg2));
OperandBundleDef OpBundle("jl_roots", gc_uses);
auto answer = ctx.builder.CreateCall(prepare_call(memcmp_func), {
ctx.builder.CreateBitCast(varg1, getInt8PtrTy(ctx.builder.getContext())),
ctx.builder.CreateBitCast(varg2, getInt8PtrTy(ctx.builder.getContext())),
ConstantInt::get(ctx.types().T_size, sz) },
ArrayRef<OperandBundleDef>(&OpBundle, nroots ? 1 : 0));
ArrayRef<OperandBundleDef>(&OpBundle, gc_uses.empty() ? 0 : 1));

if (arg1.tbaa || arg2.tbaa) {
jl_aliasinfo_t ai;
Expand Down Expand Up @@ -6432,9 +6429,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
}
SmallVector<Value*, 0> vals;
for (size_t i = 0; i < nargs; ++i) {
Value *gc_root = get_gc_root_for(ctx, argv[i]);
if (gc_root)
vals.push_back(gc_root);
vals.append(get_gc_roots_for(ctx, argv[i]));
}
Value *token = vals.empty()
? (Value*)ConstantTokenNone::get(ctx.builder.getContext())
Expand Down
20 changes: 11 additions & 9 deletions src/llvm-late-gc-lowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
#include <llvm/ADT/SetVector.h>
#include <llvm/ADT/SmallVector.h>
#include <llvm/ADT/SmallSet.h>
#include "llvm/Analysis/CFG.h"
#include <llvm/Analysis/CFG.h>
#include <llvm/Analysis/InstSimplifyFolder.h>
#include <llvm/IR/Value.h>
#include <llvm/IR/Constants.h>
#include <llvm/IR/Dominators.h>
Expand Down Expand Up @@ -455,7 +456,6 @@ SmallVector<SmallVector<unsigned, 0>, 0> TrackCompositeType(Type *T) {
}



// Walk through simple expressions to until we hit something that requires root numbering
// If the input value is a scalar (pointer), we may return a composite value as base
// in which case the second member of the pair is the index of the value in the vector.
Expand Down Expand Up @@ -616,12 +616,12 @@ Value *LateLowerGCFrame::MaybeExtractScalar(State &S, std::pair<Value*,int> ValE
V = ConstantPointerNull::get(cast<PointerType>(T_prjlvalue));
return V;
}
IRBuilder<InstSimplifyFolder> foldbuilder(InsertBefore->getContext(), InstSimplifyFolder(InsertBefore->getModule()->getDataLayout()));
foldbuilder.SetInsertPoint(InsertBefore);
if (Idxs.size() > IsVector)
V = ExtractValueInst::Create(V, IsVector ? IdxsNotVec : Idxs, "", InsertBefore);
V = foldbuilder.CreateExtractValue(V, IsVector ? IdxsNotVec : Idxs);
if (IsVector)
V = ExtractElementInst::Create(V,
ConstantInt::get(Type::getInt32Ty(V->getContext()), Idxs.back()),
"", InsertBefore);
V = foldbuilder.CreateExtractElement(V, ConstantInt::get(Type::getInt32Ty(V->getContext()), Idxs.back()));
}
return V;
}
Expand Down Expand Up @@ -1817,11 +1817,13 @@ static Value *ExtractScalar(Value *V, Type *VTy, bool isptr, ArrayRef<unsigned>
auto IdxsNotVec = Idxs.slice(0, Idxs.size() - 1);
Type *FinalT = ExtractValueInst::getIndexedType(V->getType(), IdxsNotVec);
bool IsVector = isa<VectorType>(FinalT);
IRBuilder<InstSimplifyFolder> foldbuilder(irbuilder.getContext(), InstSimplifyFolder(irbuilder.GetInsertBlock()->getModule()->getDataLayout()));
foldbuilder.restoreIP(irbuilder.saveIP());
foldbuilder.SetCurrentDebugLocation(irbuilder.getCurrentDebugLocation());
if (Idxs.size() > IsVector)
V = irbuilder.Insert(ExtractValueInst::Create(V, IsVector ? IdxsNotVec : Idxs));
V = foldbuilder.CreateExtractValue(V, IsVector ? IdxsNotVec : Idxs);
if (IsVector)
V = irbuilder.Insert(ExtractElementInst::Create(V,
ConstantInt::get(Type::getInt32Ty(V->getContext()), Idxs.back())));
V = foldbuilder.CreateExtractElement(V, ConstantInt::get(Type::getInt32Ty(V->getContext()), Idxs.back()));
}
return V;
}
Expand Down

0 comments on commit 715c50d

Please sign in to comment.