Skip to content

Commit 1eabe90

Browse files
vtjnashvtjnash
authored
codegen: some cleanup of layout computations (#55730)
Change Alloca to take an explicit alignment, rather than relying on LLVM to guess our intended alignment from the DataLayout. Eventually we should try to change this code to just get all layout data from julia queries (jl_field_offset, julia_alignment, etc.) instead of relying on creating an LLVM element type for memory and inspecting it (CountTrackedPointers, DataLayout, and so on).
1 parent 255162c commit 1eabe90

File tree

4 files changed

+44
-54
lines changed

4 files changed

+44
-54
lines changed

‎src/ccall.cpp‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -446,15 +446,13 @@ static Value *llvm_type_rewrite(
446446
const DataLayout &DL = ctx.builder.GetInsertBlock()->getModule()->getDataLayout();
447447
Align align = std::max(DL.getPrefTypeAlign(target_type), DL.getPrefTypeAlign(from_type));
448448
if (DL.getTypeAllocSize(target_type) >= DL.getTypeAllocSize(from_type)) {
449-
to = emit_static_alloca(ctx, target_type);
449+
to = emit_static_alloca(ctx, target_type, align);
450450
setName(ctx.emission_context, to, "type_rewrite_buffer");
451-
cast<AllocaInst>(to)->setAlignment(align);
452451
from = to;
453452
}
454453
else {
455-
from = emit_static_alloca(ctx, from_type);
454+
from = emit_static_alloca(ctx, from_type, align);
456455
setName(ctx.emission_context, from, "type_rewrite_buffer");
457-
cast<AllocaInst>(from)->setAlignment(align);
458456
to = from;
459457
}
460458
ctx.builder.CreateAlignedStore(v, from, align);
@@ -555,9 +553,8 @@ static Value *julia_to_native(
555553

556554
// pass the address of an alloca'd thing, not a box
557555
// since those are immutable.
558-
Value *slot = emit_static_alloca(ctx, to);
559556
Align align(julia_alignment(jlto));
560-
cast<AllocaInst>(slot)->setAlignment(align);
557+
Value *slot = emit_static_alloca(ctx, to, align);
561558
setName(ctx.emission_context, slot, "native_convert_buffer");
562559
if (!jvinfo.ispointer()) {
563560
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, jvinfo.tbaa);
@@ -2090,7 +2087,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
20902087
if (sret) {
20912088
assert(!retboxed && jl_is_datatype(rt) && "sret return type invalid");
20922089
if (jl_is_pointerfree(rt)) {
2093-
result = emit_static_alloca(ctx, lrt);
2090+
result = emit_static_alloca(ctx, lrt, Align(julia_alignment(rt)));
20942091
setName(ctx.emission_context, result, "ccall_sret");
20952092
sretty = lrt;
20962093
argvals[0] = result;
@@ -2266,9 +2263,8 @@ jl_cgval_t function_sig_t::emit_a_ccall(
22662263
if (DL.getTypeStoreSize(resultTy) > rtsz) {
22672264
// ARM and AArch64 can use a LLVM type larger than the julia type.
22682265
// When this happens, cast through memory.
2269-
auto slot = emit_static_alloca(ctx, resultTy);
2266+
auto slot = emit_static_alloca(ctx, resultTy, boxalign);
22702267
setName(ctx.emission_context, slot, "type_pun_slot");
2271-
slot->setAlignment(boxalign);
22722268
ctx.builder.CreateAlignedStore(result, slot, boxalign);
22732269
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa);
22742270
emit_memcpy(ctx, strct, ai, slot, ai, rtsz, boxalign, boxalign);

‎src/cgutils.cpp‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,18 +1937,22 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
19371937
ctx.builder.CreateFence(Order);
19381938
return ghostValue(ctx, jltype);
19391939
}
1940+
if (isboxed)
1941+
alignment = sizeof(void*);
1942+
else if (!alignment)
1943+
alignment = julia_alignment(jltype);
19401944
unsigned nb = isboxed ? sizeof(void*) : jl_datatype_size(jltype);
19411945
// note that nb == jl_Module->getDataLayout().getTypeAllocSize(elty) or getTypeStoreSize, depending on whether it is a struct or primitive type
19421946
AllocaInst *intcast = NULL;
19431947
if (Order == AtomicOrdering::NotAtomic) {
19441948
if (!isboxed && !aliasscope && elty->isAggregateType() && !CountTrackedPointers(elty).count) {
1945-
intcast = emit_static_alloca(ctx, elty);
1949+
intcast = emit_static_alloca(ctx, elty, Align(alignment));
19461950
setName(ctx.emission_context, intcast, "aggregate_load_box");
19471951
}
19481952
}
19491953
else {
19501954
if (!isboxed && !elty->isIntOrPtrTy()) {
1951-
intcast = emit_static_alloca(ctx, elty);
1955+
intcast = emit_static_alloca(ctx, elty, Align(alignment));
19521956
setName(ctx.emission_context, intcast, "atomic_load_box");
19531957
elty = Type::getIntNTy(ctx.builder.getContext(), 8 * nb);
19541958
}
@@ -1963,10 +1967,6 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
19631967
if (idx_0based)
19641968
data = ctx.builder.CreateInBoundsGEP(elty, data, idx_0based);
19651969
Value *instr = nullptr;
1966-
if (isboxed)
1967-
alignment = sizeof(void*);
1968-
else if (!alignment)
1969-
alignment = julia_alignment(jltype);
19701970
if (intcast && Order == AtomicOrdering::NotAtomic) {
19711971
emit_memcpy(ctx, intcast, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), data, jl_aliasinfo_t::fromTBAA(ctx, tbaa), nb, Align(alignment), intcast->getAlign());
19721972
}
@@ -2053,6 +2053,10 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
20532053
ret = update_julia_type(ctx, ret, jltype);
20542054
return ret;
20552055
};
2056+
if (isboxed)
2057+
alignment = sizeof(void*);
2058+
else if (!alignment)
2059+
alignment = julia_alignment(jltype);
20562060
Type *elty = isboxed ? ctx.types().T_prjlvalue : julia_type_to_llvm(ctx, jltype);
20572061
if (type_is_ghost(elty) ||
20582062
(issetfieldonce && !maybe_null_if_boxed) ||
@@ -2095,7 +2099,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
20952099
intcast_eltyp = elty;
20962100
elty = Type::getIntNTy(ctx.builder.getContext(), 8 * nb);
20972101
if (!issetfield) {
2098-
intcast = emit_static_alloca(ctx, elty);
2102+
intcast = emit_static_alloca(ctx, elty, Align(alignment));
20992103
setName(ctx.emission_context, intcast, "atomic_store_box");
21002104
}
21012105
}
@@ -2121,10 +2125,6 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
21212125
if (realelty != elty)
21222126
r = ctx.builder.CreateZExt(r, elty);
21232127
}
2124-
if (isboxed)
2125-
alignment = sizeof(void*);
2126-
else if (!alignment)
2127-
alignment = julia_alignment(jltype);
21282128
Value *instr = nullptr;
21292129
Value *Compare = nullptr;
21302130
Value *Success = nullptr;
@@ -2657,10 +2657,8 @@ static jl_cgval_t emit_unionload(jl_codectx_t &ctx, Value *addr, Value *ptindex,
26572657
if (fsz > 0 && mutabl) {
26582658
// move value to an immutable stack slot (excluding tindex)
26592659
Type *AT = ArrayType::get(IntegerType::get(ctx.builder.getContext(), 8 * al), (fsz + al - 1) / al);
2660-
AllocaInst *lv = emit_static_alloca(ctx, AT);
2660+
AllocaInst *lv = emit_static_alloca(ctx, AT, Align(al));
26612661
setName(ctx.emission_context, lv, "immutable_union");
2662-
if (al > 1)
2663-
lv->setAlignment(Align(al));
26642662
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa);
26652663
emit_memcpy(ctx, lv, ai, addr, ai, fsz, Align(al), Align(al));
26662664
addr = lv;
@@ -2903,7 +2901,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
29032901
unsigned st_idx = convert_struct_offset(ctx, T, byte_offset);
29042902
IntegerType *ET = cast<IntegerType>(T->getStructElementType(st_idx));
29052903
unsigned align = (ET->getBitWidth() + 7) / 8;
2906-
lv = emit_static_alloca(ctx, ET);
2904+
lv = emit_static_alloca(ctx, ET, Align(align));
29072905
lv->setOperand(0, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), (fsz + align - 1) / align));
29082906
// emit all of the align-sized words
29092907
unsigned i = 0;
@@ -3324,10 +3322,8 @@ static AllocaInst *try_emit_union_alloca(jl_codectx_t &ctx, jl_uniontype_t *ut,
33243322
// at least some of the values can live on the stack
33253323
// try to pick an Integer type size such that SROA will emit reasonable code
33263324
Type *AT = ArrayType::get(IntegerType::get(ctx.builder.getContext(), 8 * min_align), (nbytes + min_align - 1) / min_align);
3327-
AllocaInst *lv = emit_static_alloca(ctx, AT);
3325+
AllocaInst *lv = emit_static_alloca(ctx, AT, Align(align));
33283326
setName(ctx.emission_context, lv, "unionalloca");
3329-
if (align > 1)
3330-
lv->setAlignment(Align(align));
33313327
return lv;
33323328
}
33333329
return NULL;
@@ -3886,7 +3882,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
38863882
}
38873883
}
38883884
else {
3889-
strct = emit_static_alloca(ctx, lt);
3885+
strct = emit_static_alloca(ctx, lt, Align(julia_alignment(ty)));
38903886
setName(ctx.emission_context, strct, arg_typename);
38913887
if (tracked.count)
38923888
undef_derived_strct(ctx, strct, sty, ctx.tbaa().tbaa_stack);
@@ -3966,7 +3962,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
39663962
if (fsz1 > 0 && !fval_info.isghost) {
39673963
Type *ET = IntegerType::get(ctx.builder.getContext(), 8 * al);
39683964
assert(lt->getStructElementType(llvm_idx) == ET);
3969-
AllocaInst *lv = emit_static_alloca(ctx, ET);
3965+
AllocaInst *lv = emit_static_alloca(ctx, ET, Align(al));
39703966
setName(ctx.emission_context, lv, "unioninit");
39713967
lv->setOperand(0, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), (fsz1 + al - 1) / al));
39723968
emit_unionmove(ctx, lv, ctx.tbaa().tbaa_stack, fval_info, nullptr);

‎src/codegen.cpp‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2208,10 +2208,10 @@ static GlobalVariable *get_pointer_to_constant(jl_codegen_params_t &emission_con
22082208
return gv;
22092209
}
22102210

2211-
static AllocaInst *emit_static_alloca(jl_codectx_t &ctx, Type *lty)
2211+
static AllocaInst *emit_static_alloca(jl_codectx_t &ctx, Type *lty, Align align)
22122212
{
22132213
++EmittedAllocas;
2214-
return new AllocaInst(lty, ctx.topalloca->getModule()->getDataLayout().getAllocaAddrSpace(), "", /*InsertBefore=*/ctx.topalloca);
2214+
return new AllocaInst(lty, ctx.topalloca->getModule()->getDataLayout().getAllocaAddrSpace(), nullptr, align, "", /*InsertBefore=*/ctx.topalloca);
22152215
}
22162216

22172217
static void undef_derived_strct(jl_codectx_t &ctx, Value *ptr, jl_datatype_t *sty, MDNode *tbaa)
@@ -2323,7 +2323,7 @@ static inline jl_cgval_t value_to_pointer(jl_codectx_t &ctx, Value *v, jl_value_
23232323
loc = get_pointer_to_constant(ctx.emission_context, cast<Constant>(v), Align(julia_alignment(typ)), "_j_const", *jl_Module);
23242324
}
23252325
else {
2326-
loc = emit_static_alloca(ctx, v->getType());
2326+
loc = emit_static_alloca(ctx, v->getType(), Align(julia_alignment(typ)));
23272327
ctx.builder.CreateStore(v, loc);
23282328
}
23292329
return mark_julia_slot(loc, typ, tindex, ctx.tbaa().tbaa_stack);
@@ -2435,7 +2435,7 @@ static void alloc_def_flag(jl_codectx_t &ctx, jl_varinfo_t& vi)
24352435
{
24362436
assert((!vi.boxroot || vi.pTIndex) && "undef check is null pointer for boxed things");
24372437
if (vi.usedUndef) {
2438-
vi.defFlag = emit_static_alloca(ctx, getInt1Ty(ctx.builder.getContext()));
2438+
vi.defFlag = emit_static_alloca(ctx, getInt1Ty(ctx.builder.getContext()), Align(1));
24392439
setName(ctx.emission_context, vi.defFlag, "isdefined");
24402440
store_def_flag(ctx, vi, false);
24412441
}
@@ -5025,25 +5025,20 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, bool is_opaque_clos
50255025
case jl_returninfo_t::Ghosts:
50265026
break;
50275027
case jl_returninfo_t::SRet:
5028-
result = emit_static_alloca(ctx, getAttributeAtIndex(returninfo.attrs, 1, Attribute::StructRet).getValueAsType());
5029-
#if JL_LLVM_VERSION < 170000
5030-
assert(cast<PointerType>(result->getType())->hasSameElementTypeAs(cast<PointerType>(cft->getParamType(0))));
5031-
#endif
5028+
result = emit_static_alloca(ctx, getAttributeAtIndex(returninfo.attrs, 1, Attribute::StructRet).getValueAsType(), Align(julia_alignment(jlretty)));
50325029
argvals[idx] = result;
50335030
idx++;
50345031
break;
50355032
case jl_returninfo_t::Union:
5036-
result = emit_static_alloca(ctx, ArrayType::get(getInt8Ty(ctx.builder.getContext()), returninfo.union_bytes));
5033+
result = emit_static_alloca(ctx, ArrayType::get(getInt8Ty(ctx.builder.getContext()), returninfo.union_bytes), Align(returninfo.union_align));
50375034
setName(ctx.emission_context, result, "sret_box");
5038-
if (returninfo.union_align > 1)
5039-
result->setAlignment(Align(returninfo.union_align));
50405035
argvals[idx] = result;
50415036
idx++;
50425037
break;
50435038
}
50445039

50455040
if (returninfo.return_roots) {
5046-
AllocaInst *return_roots = emit_static_alloca(ctx, ArrayType::get(ctx.types().T_prjlvalue, returninfo.return_roots));
5041+
AllocaInst *return_roots = emit_static_alloca(ctx, ArrayType::get(ctx.types().T_prjlvalue, returninfo.return_roots), Align(alignof(jl_value_t*)));
50475042
argvals[idx] = return_roots;
50485043
idx++;
50495044
}
@@ -5922,11 +5917,10 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
59225917
if (vtype->isAggregateType() && CountTrackedPointers(vtype).count == 0) {
59235918
// the value will be moved into dest in the predecessor critical block.
59245919
// here it's moved into phi in the successor (from dest)
5925-
dest = emit_static_alloca(ctx, vtype);
5926-
Value *phi = emit_static_alloca(ctx, vtype);
5927-
ctx.builder.CreateMemCpy(phi, Align(julia_alignment(phiType)),
5928-
dest, dest->getAlign(),
5929-
jl_datatype_size(phiType), false);
5920+
Align align(julia_alignment(phiType));
5921+
dest = emit_static_alloca(ctx, vtype, align);
5922+
Value *phi = emit_static_alloca(ctx, vtype, align);
5923+
ctx.builder.CreateMemCpy(phi, align, dest, align, jl_datatype_size(phiType), false);
59305924
ctx.builder.CreateLifetimeEnd(dest);
59315925
slot = mark_julia_slot(phi, phiType, NULL, ctx.tbaa().tbaa_stack);
59325926
}
@@ -7737,6 +7731,8 @@ static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, Value
77377731
if (tracked.count && !tracked.all)
77387732
props.return_roots = tracked.count;
77397733
props.cc = jl_returninfo_t::SRet;
7734+
props.union_bytes = jl_datatype_size(jlrettype);
7735+
props.union_align = props.union_minalign = jl_datatype_align(jlrettype);
77407736
// sret is always passed from alloca
77417737
assert(M);
77427738
fsig.push_back(rt->getPointerTo(M->getDataLayout().getAllocaAddrSpace()));
@@ -8365,13 +8361,13 @@ static jl_llvm_functions_t
83658361
if (lv) {
83668362
lv->setName(jl_symbol_name(s));
83678363
varinfo.value = mark_julia_slot(lv, jt, NULL, ctx.tbaa().tbaa_stack);
8368-
varinfo.pTIndex = emit_static_alloca(ctx, getInt8Ty(ctx.builder.getContext()));
8364+
varinfo.pTIndex = emit_static_alloca(ctx, getInt8Ty(ctx.builder.getContext()), Align(1));
83698365
setName(ctx.emission_context, varinfo.pTIndex, "tindex");
83708366
// TODO: attach debug metadata to this variable
83718367
}
83728368
else if (allunbox) {
83738369
// all ghost values just need a selector allocated
8374-
AllocaInst *lv = emit_static_alloca(ctx, getInt8Ty(ctx.builder.getContext()));
8370+
AllocaInst *lv = emit_static_alloca(ctx, getInt8Ty(ctx.builder.getContext()), Align(1));
83758371
lv->setName(jl_symbol_name(s));
83768372
varinfo.pTIndex = lv;
83778373
varinfo.value.tbaa = NULL;

‎src/intrinsics.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,11 @@ static Value *emit_unboxed_coercion(jl_codectx_t &ctx, Type *to, Value *unboxed)
405405
}
406406
else if (!ty->isIntOrPtrTy() && !ty->isFloatingPointTy()) {
407407
assert(DL.getTypeSizeInBits(ty) == DL.getTypeSizeInBits(to));
408-
AllocaInst *cast = emit_static_alloca(ctx, ty);
408+
Align align = std::max(DL.getPrefTypeAlign(ty), DL.getPrefTypeAlign(to));
409+
AllocaInst *cast = emit_static_alloca(ctx, ty, align);
409410
setName(ctx.emission_context, cast, "coercion");
410-
ctx.builder.CreateStore(unboxed, cast);
411-
unboxed = ctx.builder.CreateLoad(to, cast);
411+
ctx.builder.CreateAlignedStore(unboxed, cast, align);
412+
unboxed = ctx.builder.CreateAlignedLoad(to, cast, align);
412413
}
413414
else if (frompointer) {
414415
Type *INTT_to = INTT(to, DL);
@@ -692,10 +693,11 @@ static jl_cgval_t generic_cast(
692693
// understood that everything is implicitly rounded to 23 bits,
693694
// but if we start looking at more bits we need to actually do the
694695
// rounding first instead of carrying around incorrect low bits.
695-
Value *jlfloattemp_var = emit_static_alloca(ctx, from->getType());
696+
Align align(julia_alignment((jl_value_t*)jlto));
697+
Value *jlfloattemp_var = emit_static_alloca(ctx, from->getType(), align);
696698
setName(ctx.emission_context, jlfloattemp_var, "rounding_slot");
697-
ctx.builder.CreateStore(from, jlfloattemp_var);
698-
from = ctx.builder.CreateLoad(from->getType(), jlfloattemp_var, /*force this to load from the stack*/true);
699+
ctx.builder.CreateAlignedStore(from, jlfloattemp_var, align);
700+
from = ctx.builder.CreateAlignedLoad(from->getType(), jlfloattemp_var, align, /*force this to load from the stack*/true);
699701
setName(ctx.emission_context, from, "rounded");
700702
}
701703
}

0 commit comments

Comments
 (0)