Skip to content

Commit

Permalink
Utilize correct tbaa when emitting stores of unions. (#54222)
Browse files Browse the repository at this point in the history
Also remove unused argument from union_store
  • Loading branch information
gbaraldi authored Apr 25, 2024
1 parent 8f6418e commit 37f848c
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3725,7 +3725,7 @@ static void emit_write_multibarrier(jl_codectx_t &ctx, Value *parent, Value *agg

static jl_cgval_t union_store(jl_codectx_t &ctx,
Value *ptr, Value *ptindex, jl_cgval_t rhs, jl_cgval_t cmp,
jl_value_t *jltype, MDNode *tbaa, MDNode *aliasscope, MDNode *tbaa_tindex,
jl_value_t *jltype, MDNode *tbaa, MDNode *tbaa_tindex,
AtomicOrdering Order, AtomicOrdering FailOrder,
Value *needlock, bool issetfield, bool isreplacefield, bool isswapfield, bool ismodifyfield, bool issetfieldonce,
const jl_cgval_t *modifyop, const Twine &fname)
Expand Down Expand Up @@ -3783,7 +3783,7 @@ static jl_cgval_t union_store(jl_codectx_t &ctx,
}
Value *tindex = compute_tindex_unboxed(ctx, rhs_union, jltype);
tindex = ctx.builder.CreateNUWSub(tindex, ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 1));
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_unionselbyte);
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa_tindex);
ai.decorateInst(ctx.builder.CreateAlignedStore(tindex, ptindex, Align(1)));
// copy data
if (!rhs.isghost) {
Expand Down Expand Up @@ -3839,7 +3839,7 @@ static jl_cgval_t emit_setfield(jl_codectx_t &ctx,
emit_bitcast(ctx, addr, getInt8PtrTy(ctx.builder.getContext())),
ConstantInt::get(ctx.types().T_size, fsz1));
setNameWithField(ctx.emission_context, ptindex, get_objname, sty, idx0, Twine(".tindex_ptr"));
return union_store(ctx, addr, ptindex, rhs, cmp, jfty, tbaa, nullptr, ctx.tbaa().tbaa_unionselbyte,
return union_store(ctx, addr, ptindex, rhs, cmp, jfty, tbaa, ctx.tbaa().tbaa_unionselbyte,
Order, FailOrder,
needlock, issetfield, isreplacefield, isswapfield, ismodifyfield, issetfieldonce,
modifyop, fname);
Expand Down
2 changes: 1 addition & 1 deletion src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3875,7 +3875,7 @@ static bool emit_f_opmemory(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
ptindex = emit_bitcast(ctx, ptindex, getInt8PtrTy(ctx.builder.getContext()));
ptindex = ctx.builder.CreateInBoundsGEP(getInt8Ty(ctx.builder.getContext()), ptindex, idx0);
*ret = union_store(ctx, data, ptindex, val, cmp, ety,
ctx.tbaa().tbaa_arraybuf, nullptr, ctx.tbaa().tbaa_arrayselbyte,
ctx.tbaa().tbaa_arraybuf, ctx.tbaa().tbaa_arrayselbyte,
Order, FailOrder,
nullptr, issetmemory, isreplacememory, isswapmemory, ismodifymemory, issetmemoryonce,
modifyop, fname);
Expand Down
14 changes: 14 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -873,3 +873,17 @@ if Sys.ARCH === :x86_64
end
end
end

#Check if we aren't emitting the store with the wrong TBAA metadata

foo54166(x,i,y) = x[i] = y
let io = IOBuffer()
code_llvm(io,foo54166, (Vector{Union{Missing,Int}}, Int, Int), dump_module=true, raw=true)
str = String(take!(io))
@test !occursin("jtbaa_unionselbyte", str)
@test occursin("jtbaa_arrayselbyte", str)
end

ex54166 = Union{Missing, Int64}[missing -2; missing -2];
dims54166 = (1,2)
@test (minimum(ex54166; dims=dims54166)[1] === missing)

0 comments on commit 37f848c

Please sign in to comment.