From ad4091d4b2f4c3b7f87c7ccc42104a62dd0d0b0f Mon Sep 17 00:00:00 2001 From: Prem Chintalapudi Date: Mon, 31 Jul 2023 10:59:52 -0500 Subject: [PATCH 1/5] Add alignment to constant globals --- src/cgutils.cpp | 7 ++++--- src/codegen.cpp | 11 ++++++----- src/llvm-late-gc-lowering.cpp | 5 ++--- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index aa641846498d3..ce406f775fd09 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -125,7 +125,8 @@ static Value *stringConstPtr( ctxt.resize(28); ctxt[25] = ctxt[26] = ctxt[27] = '.'; } - GlobalVariable *gv = get_pointer_to_constant(emission_context, Data, "_j_str_" + StringRef(ctxt.data(), ctxt.size()), *M); + // Doesn't need to be aligned, we shouldn't operate on these like julia objects + GlobalVariable *gv = get_pointer_to_constant(emission_context, Data, Align(1), "_j_str_" + StringRef(ctxt.data(), ctxt.size()), *M); Value *zero = ConstantInt::get(Type::getInt32Ty(irbuilder.getContext()), 0); Value *Args[] = { zero, zero }; auto gep = irbuilder.CreateInBoundsGEP(gv->getValueType(), @@ -918,7 +919,7 @@ static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x) if (x.constant) { Constant *val = julia_const_to_llvm(ctx, x.constant); if (val) - data = get_pointer_to_constant(ctx.emission_context, val, "_j_const", *jl_Module); + data = get_pointer_to_constant(ctx.emission_context, val, Align(jl_is_datatype(x.typ) && jl_struct_try_layout((jl_datatype_t*)x.typ) ? julia_alignment(x.typ) : JL_HEAP_ALIGNMENT), "_j_const", *jl_Module); else data = literal_pointer_val(ctx, x.constant); } @@ -1106,7 +1107,7 @@ static Value *emit_typeof(jl_codectx_t &ctx, const jl_cgval_t &p, bool maybenull if (justtag && jt->smalltag) { ptr = ConstantInt::get(expr_type, jt->smalltag << 4); if (ctx.emission_context.imaging) - ptr = get_pointer_to_constant(ctx.emission_context, ptr, StringRef("_j_smalltag_") + jl_symbol_name(jt->name->name), *jl_Module); + ptr = get_pointer_to_constant(ctx.emission_context, ptr, Align(julia_alignment((jl_value_t *)jl_datatype_type)), StringRef("_j_smalltag_") + jl_symbol_name(jt->name->name), *jl_Module); } else if (ctx.emission_context.imaging) ptr = ConstantExpr::getBitCast(literal_pointer_val_slot(ctx, (jl_value_t*)jt), datatype_or_p->getType()); diff --git a/src/codegen.cpp b/src/codegen.cpp index 5b0bf1bc98876..b3ad61e86d061 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1776,7 +1776,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const jl_cgval_t *argv, size_t nargs, jl_value_t *rt); static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p); -static GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G); +static unsigned julia_alignment(jl_value_t *jt); static GlobalVariable *prepare_global_in(Module *M, JuliaVariable *G) { @@ -1811,7 +1811,7 @@ static inline GlobalVariable *prepare_global_in(Module *M, GlobalVariable *G) // --- convenience functions for tagging llvm values with julia types --- -static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_context, Constant *val, const Twine &name, Module &M) +static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_context, Constant *val, Align align, const Twine &name, Module &M) { GlobalVariable *&gv = emission_context.mergedConstants[val]; auto get_gv = [&](const Twine &name) { @@ -1823,6 +1823,7 @@ static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_con val, name); gv->setUnnamedAddr(GlobalValue::UnnamedAddr::Global); + gv->setAlignment(align); return gv; }; if (gv == nullptr) { @@ -1943,7 +1944,7 @@ static inline jl_cgval_t value_to_pointer(jl_codectx_t &ctx, Value *v, jl_value_ { Value *loc; if (valid_as_globalinit(v)) { // llvm can't handle all the things that could be inside a ConstantExpr - loc = get_pointer_to_constant(ctx.emission_context, cast(v), "_j_const", *jl_Module); + loc = get_pointer_to_constant(ctx.emission_context, cast(v), Align(jl_is_datatype(typ) && jl_struct_try_layout((jl_datatype_t*)typ) ? julia_alignment(typ) : JL_HEAP_ALIGNMENT), "_j_const", *jl_Module); } else { loc = emit_static_alloca(ctx, v->getType()); @@ -7663,7 +7664,7 @@ static jl_llvm_functions_t Type *vtype = julia_type_to_llvm(ctx, jt, &isboxed); assert(!isboxed); assert(!type_is_ghost(vtype) && "constants should already be handled"); - Value *lv = new AllocaInst(vtype, M->getDataLayout().getAllocaAddrSpace(), NULL, Align(jl_datatype_align(jt)), jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); + Value *lv = new AllocaInst(vtype, M->getDataLayout().getAllocaAddrSpace(), nullptr, Align(jl_datatype_align(jt)), jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); if (CountTrackedPointers(vtype).count) { StoreInst *SI = new StoreInst(Constant::getNullValue(vtype), lv, false, Align(sizeof(void*))); SI->insertAfter(ctx.topalloca); @@ -7683,7 +7684,7 @@ static jl_llvm_functions_t (va && (int)i == ctx.vaSlot) || // or it's the va arg tuple i == 0) { // or it is the first argument (which isn't in `argArray`) AllocaInst *av = new AllocaInst(ctx.types().T_prjlvalue, M->getDataLayout().getAllocaAddrSpace(), - jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); + nullptr, Align(jl_is_datatype(jt) && jl_struct_try_layout((jl_datatype_t*)jt) ? julia_alignment(jt) : JL_HEAP_ALIGNMENT), jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); StoreInst *SI = new StoreInst(Constant::getNullValue(ctx.types().T_prjlvalue), av, false, Align(sizeof(void*))); SI->insertAfter(ctx.topalloca); varinfo.boxroot = av; diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 409d3a09c0337..c56a7beb16086 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -2386,8 +2386,7 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) { if (isLoadFromConstGV(LI, task_local) && getLoadValueAlign(LI) < 16) { Type *T_int64 = Type::getInt64Ty(LI->getContext()); auto op = ConstantAsMetadata::get(ConstantInt::get(T_int64, 16)); - LI->setMetadata(LLVMContext::MD_align, - MDNode::get(LI->getContext(), { op })); + LI->setMetadata(LLVMContext::MD_align, MDNode::get(LI->getContext(), { op })); } } // As a last resort, if we didn't manage to strip down the tag @@ -2468,7 +2467,7 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) { } ReplacementArgs.push_back(nframeargs == 0 ? (llvm::Value*)ConstantPointerNull::get(T_pprjlvalue) : - (allocaAddressSpace ? Builder.CreateAddrSpaceCast(Frame, T_prjlvalue->getPointerTo(0)) : Frame)); + Builder.CreateAddrSpaceCast(Frame, T_prjlvalue->getPointerTo(0))); ReplacementArgs.push_back(ConstantInt::get(T_int32, nframeargs)); if (callee == call2_func) { // move trailing arg to the end now From bef57372a78197f31104e982947b5f9b2332bd96 Mon Sep 17 00:00:00 2001 From: pchintalapudi <34727397+pchintalapudi@users.noreply.github.com> Date: Mon, 31 Jul 2023 17:56:13 +0000 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Jameson Nash --- src/cgutils.cpp | 4 ++-- src/codegen.cpp | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index ce406f775fd09..bcf1568956a43 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -919,7 +919,7 @@ static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x) if (x.constant) { Constant *val = julia_const_to_llvm(ctx, x.constant); if (val) - data = get_pointer_to_constant(ctx.emission_context, val, Align(jl_is_datatype(x.typ) && jl_struct_try_layout((jl_datatype_t*)x.typ) ? julia_alignment(x.typ) : JL_HEAP_ALIGNMENT), "_j_const", *jl_Module); + data = get_pointer_to_constant(ctx.emission_context, val, Align(julia_alignment((jl_datatype_t*)jl_typeof(x.constant))), "_j_const", *jl_Module); else data = literal_pointer_val(ctx, x.constant); } @@ -1107,7 +1107,7 @@ static Value *emit_typeof(jl_codectx_t &ctx, const jl_cgval_t &p, bool maybenull if (justtag && jt->smalltag) { ptr = ConstantInt::get(expr_type, jt->smalltag << 4); if (ctx.emission_context.imaging) - ptr = get_pointer_to_constant(ctx.emission_context, ptr, Align(julia_alignment((jl_value_t *)jl_datatype_type)), StringRef("_j_smalltag_") + jl_symbol_name(jt->name->name), *jl_Module); + ptr = get_pointer_to_constant(ctx.emission_context, ptr, Align(sizeof(jl_value_t*)), StringRef("_j_smalltag_") + jl_symbol_name(jt->name->name), *jl_Module); } else if (ctx.emission_context.imaging) ptr = ConstantExpr::getBitCast(literal_pointer_val_slot(ctx, (jl_value_t*)jt), datatype_or_p->getType()); diff --git a/src/codegen.cpp b/src/codegen.cpp index b3ad61e86d061..51ded890c063a 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1944,7 +1944,8 @@ static inline jl_cgval_t value_to_pointer(jl_codectx_t &ctx, Value *v, jl_value_ { Value *loc; if (valid_as_globalinit(v)) { // llvm can't handle all the things that could be inside a ConstantExpr - loc = get_pointer_to_constant(ctx.emission_context, cast(v), Align(jl_is_datatype(typ) && jl_struct_try_layout((jl_datatype_t*)typ) ? julia_alignment(typ) : JL_HEAP_ALIGNMENT), "_j_const", *jl_Module); + assert(jl_is_concrete_type(typ)); // not legal to have an unboxed abstract type + loc = get_pointer_to_constant(ctx.emission_context, cast(v), Align(julia_alignment(typ)), "_j_const", *jl_Module); } else { loc = emit_static_alloca(ctx, v->getType()); @@ -7684,7 +7685,7 @@ static jl_llvm_functions_t (va && (int)i == ctx.vaSlot) || // or it's the va arg tuple i == 0) { // or it is the first argument (which isn't in `argArray`) AllocaInst *av = new AllocaInst(ctx.types().T_prjlvalue, M->getDataLayout().getAllocaAddrSpace(), - nullptr, Align(jl_is_datatype(jt) && jl_struct_try_layout((jl_datatype_t*)jt) ? julia_alignment(jt) : JL_HEAP_ALIGNMENT), jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); + nullptr, Align(sizeof(jl_value_t*)), jl_symbol_name(s), /*InsertBefore*/ctx.topalloca); StoreInst *SI = new StoreInst(Constant::getNullValue(ctx.types().T_prjlvalue), av, false, Align(sizeof(void*))); SI->insertAfter(ctx.topalloca); varinfo.boxroot = av; From 3bcc901934a5e2c1d1b078ad0736120e5cfd24b6 Mon Sep 17 00:00:00 2001 From: pchintalapudi <34727397+pchintalapudi@users.noreply.github.com> Date: Mon, 31 Jul 2023 18:03:23 +0000 Subject: [PATCH 3/5] Update src/cgutils.cpp --- src/cgutils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index bcf1568956a43..20c312c10a5a2 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -919,7 +919,7 @@ static Value *data_pointer(jl_codectx_t &ctx, const jl_cgval_t &x) if (x.constant) { Constant *val = julia_const_to_llvm(ctx, x.constant); if (val) - data = get_pointer_to_constant(ctx.emission_context, val, Align(julia_alignment((jl_datatype_t*)jl_typeof(x.constant))), "_j_const", *jl_Module); + data = get_pointer_to_constant(ctx.emission_context, val, Align(julia_alignment(jl_typeof(x.constant))), "_j_const", *jl_Module); else data = literal_pointer_val(ctx, x.constant); } From fda2a312aa8afacdbee9c2a6e6b6be9677c0f52e Mon Sep 17 00:00:00 2001 From: Prem Chintalapudi Date: Tue, 1 Aug 2023 14:56:32 -0500 Subject: [PATCH 4/5] Fix value_to_pointer call in convert_julia_type Co-authored-by: Jameson Nash --- src/codegen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 51ded890c063a..32aee1613716d 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -2319,7 +2319,7 @@ static jl_cgval_t convert_julia_type(jl_codectx_t &ctx, const jl_cgval_t &v, jl_ new_tindex = ConstantInt::get(getInt8Ty(ctx.builder.getContext()), new_idx); if (v.V && !v.ispointer()) { // TODO: remove this branch once all consumers of v.TIndex understand how to handle a non-ispointer value - return value_to_pointer(ctx, v.V, typ, new_tindex); + return jl_cgval_t(value_to_pointer(ctx, v.V, v.typ), typ, new_tindex); } } else if (jl_subtype(v.typ, typ)) { From c2fb4e8b1edb6cd62b5c987673bc3847ea95400d Mon Sep 17 00:00:00 2001 From: Prem Chintalapudi Date: Tue, 1 Aug 2023 16:06:22 -0500 Subject: [PATCH 5/5] Fix compilation error --- src/codegen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 32aee1613716d..9ec9d1133b9ad 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -2319,7 +2319,7 @@ static jl_cgval_t convert_julia_type(jl_codectx_t &ctx, const jl_cgval_t &v, jl_ new_tindex = ConstantInt::get(getInt8Ty(ctx.builder.getContext()), new_idx); if (v.V && !v.ispointer()) { // TODO: remove this branch once all consumers of v.TIndex understand how to handle a non-ispointer value - return jl_cgval_t(value_to_pointer(ctx, v.V, v.typ), typ, new_tindex); + return jl_cgval_t(value_to_pointer(ctx, v), typ, new_tindex); } } else if (jl_subtype(v.typ, typ)) {