Skip to content

Commit

Permalink
codegen: fix bad declaration of memcmp stdlib function signature (#34792
Browse files Browse the repository at this point in the history 
)

We were lying to LLVM, and it now catches that in the verifier. Stop
lying to LLVM so that it will again be happy to generate good code for
us (e.g. optimizing this to bcmp).
  • Loading branch information
@vtjnash
vtjnash authored Feb 19, 2020
1 parent f36edc2 commit d634f73
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 38 deletions.
20 changes: 11 additions & 9 deletions src/ccall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1390,9 +1390,9 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
if (jl_is_long(argi_root))
continue;
jl_cgval_t arg_root = emit_expr(ctx, argi_root);
if (arg_root.Vboxed || arg_root.V) {
gc_uses.push_back(arg_root.Vboxed ? arg_root.Vboxed : arg_root.V);
}
Value *gc_root = get_gc_root_for(arg_root);
if (gc_root)
gc_uses.push_back(gc_root);
}

jl_unionall_t *unionall = (jl_is_method(ctx.linfo->def.method) && jl_is_unionall(ctx.linfo->def.method->sig))
Expand Down Expand Up @@ -1487,16 +1487,17 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
if (!retboxed) {
return mark_or_box_ccall_result(
ctx,
emit_pointer_from_objref(ctx,
emit_bitcast(ctx, ary, T_prjlvalue)),
ctx.builder.CreatePtrToInt(
emit_pointer_from_objref(ctx, emit_bitcast(ctx, ary, T_prjlvalue)),
T_size),
retboxed, rt, unionall, static_rt);
}
else {
return mark_or_box_ccall_result(
ctx,
ctx.builder.CreateAddrSpaceCast(
ctx.builder.CreateIntToPtr(ary, T_pjlvalue),
T_prjlvalue), // TODO: this addrspace cast is invalid (implies that the value is rooted elsewhere)
T_prjlvalue), // WARNING: this addrspace cast necessarily implies that the value is rooted elsewhere!
retboxed, rt, unionall, static_rt);
}
}
Expand Down Expand Up @@ -1647,7 +1648,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
else if (is_libjulia_func(jl_string_ptr)) {
assert(lrt == T_size);
assert(!isVa && !llvmcall && nccallargs == 1);
Value *obj = emit_pointer_from_objref(ctx, boxed(ctx, argv[0]));
Value *obj = ctx.builder.CreatePtrToInt(emit_pointer_from_objref(ctx, boxed(ctx, argv[0])), T_size);
Value *strp = ctx.builder.CreateAdd(obj, ConstantInt::get(T_size, sizeof(void*)));
JL_GC_POP();
return mark_or_box_ccall_result(ctx, strp, retboxed, rt, unionall, static_rt);
Expand All @@ -1662,7 +1663,8 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
ctx.builder.CreateIntToPtr(destp, T_pint8),
1,
ctx.builder.CreateIntToPtr(
emit_unbox(ctx, T_size, src, (jl_value_t*)jl_voidpointer_type), T_pint8),
emit_unbox(ctx, T_size, src, (jl_value_t*)jl_voidpointer_type),
T_pint8),
0,
emit_unbox(ctx, T_size, n, (jl_value_t*)jl_ulong_type),
false);
Expand Down Expand Up @@ -1789,7 +1791,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
literal_pointer_val(ctx, (jl_value_t*)rt));
sretboxed = true;
gc_uses.push_back(result);
argvals[0] = ctx.builder.CreateIntToPtr(emit_pointer_from_objref(ctx, result), fargt_sig.at(0));
argvals[0] = ctx.builder.CreateBitCast(emit_pointer_from_objref(ctx, result), fargt_sig.at(0));
}
}

Expand Down
36 changes: 23 additions & 13 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,24 +253,34 @@ static DIType *julia_type_to_di(jl_value_t *jt, DIBuilder *dbuilder, bool isboxe
return (llvm::DIType*)jdt->ditype;
}

static Value *emit_pointer_from_objref_internal(jl_codectx_t &ctx, Value *V)
static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
{
unsigned AS = cast<PointerType>(V->getType())->getAddressSpace();
if (AS != AddressSpace::Tracked && AS != AddressSpace::Derived)
return V;
V = decay_derived(V);
Type *T = PointerType::get(T_jlvalue, AddressSpace::Derived);
if (V->getType() != T)
V = ctx.builder.CreateBitCast(V, T);
CallInst *Call = ctx.builder.CreateCall(prepare_call(pointer_from_objref_func), V);
Call->addAttribute(AttributeList::FunctionIndex, Attribute::ReadNone);
Call->setAttributes(pointer_from_objref_func->getAttributes());
return Call;
}

static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
static Value *get_gc_root_for(const jl_cgval_t &x)
{
unsigned AS = cast<PointerType>(V->getType())->getAddressSpace();
if (AS != AddressSpace::Tracked && AS != AddressSpace::Derived)
return ctx.builder.CreatePtrToInt(V, T_size);
V = ctx.builder.CreateBitCast(decay_derived(V),
PointerType::get(T_jlvalue, AddressSpace::Derived));

return ctx.builder.CreatePtrToInt(
emit_pointer_from_objref_internal(ctx, V),
T_size);
if (x.Vboxed)
return x.Vboxed;
if (x.ispointer() && !x.constant) {
assert(x.V);
if (PointerType *T = dyn_cast<PointerType>(x.V->getType())) {
if (T->getAddressSpace() == AddressSpace::Tracked ||
T->getAddressSpace() == AddressSpace::Derived) {
return x.V;
}
}
}
return nullptr;
}

// --- emitting pointers directly into code ---
Expand Down Expand Up @@ -1896,7 +1906,7 @@ static Value *emit_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isb
static Value *emit_unsafe_arrayptr(jl_codectx_t &ctx, const jl_cgval_t &tinfo, bool isboxed = false)
{
Value *t = boxed(ctx, tinfo);
t = emit_pointer_from_objref_internal(ctx, decay_derived(t));
t = emit_pointer_from_objref(ctx, decay_derived(t));
return emit_arrayptr_internal(ctx, tinfo, t, 0, isboxed);
}

Expand Down
42 changes: 27 additions & 15 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ static Function *jlsubtype_func;
static Function *jlapplytype_func;
static Function *jl_object_id__func;
static Function *setjmp_func;
static Function *memcmp_derived_func;
static Function *memcmp_func;
static Function *box_int8_func;
static Function *box_uint8_func;
static Function *box_int16_func;
Expand Down Expand Up @@ -2394,13 +2394,24 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t a
Value *varg1 = arg1.ispointer() ? maybe_decay_tracked(data_pointer(ctx, arg1)) : arg1.V;
Value *varg2 = arg2.ispointer() ? maybe_decay_tracked(data_pointer(ctx, arg2)) : arg2.V;
if (sz > 512 && !sty->layout->haspadding) {
varg1 = decay_derived(arg1.ispointer() ? varg1 : value_to_pointer(ctx, arg1).V);
varg2 = decay_derived(arg2.ispointer() ? varg2 : value_to_pointer(ctx, arg2).V);
Value *answer = ctx.builder.CreateCall(prepare_call(memcmp_derived_func), {
maybe_bitcast(ctx, varg1, T_pint8),
maybe_bitcast(ctx, varg2, T_pint8),
ConstantInt::get(T_size, sz)
});
if (!arg1.ispointer())
varg1 = value_to_pointer(ctx, arg1).V;
if (!arg2.ispointer())
varg2 = 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;
if ((gc_uses[nroots] = get_gc_root_for(arg1)))
nroots++;
if ((gc_uses[nroots] = get_gc_root_for(arg2)))
nroots++;
OperandBundleDef OpBundle("jl_roots", gc_uses);
Value *answer = ctx.builder.CreateCall(prepare_call(memcmp_func), {
ctx.builder.CreateBitCast(varg1, T_pint8),
ctx.builder.CreateBitCast(varg2, T_pint8),
ConstantInt::get(T_size, sz) },
ArrayRef<OperandBundleDef>(&OpBundle, nroots ? 1 : 0));
return ctx.builder.CreateICmpEQ(answer, ConstantInt::get(T_int32, 0));
}
else {
Expand Down Expand Up @@ -7329,16 +7340,17 @@ static void init_julia_llvm_env(Module *m)
add_named_global(setjmp_func, &jl_setjmp_f);

std::vector<Type*> args_memcmp(0);
args_memcmp.push_back(T_pint8_derived);
args_memcmp.push_back(T_pint8_derived);
args_memcmp.push_back(T_pint8);
args_memcmp.push_back(T_pint8);
args_memcmp.push_back(T_size);
memcmp_derived_func =
memcmp_func =
Function::Create(FunctionType::get(T_int32, args_memcmp, false),
Function::ExternalLinkage, "memcmp", m);
memcmp_derived_func->addFnAttr(Attribute::ReadOnly);
memcmp_derived_func->addFnAttr(Attribute::NoUnwind);
memcmp_derived_func->addFnAttr(Attribute::ArgMemOnly);
add_named_global(memcmp_derived_func, &memcmp);
memcmp_func->addFnAttr(Attribute::ReadOnly);
memcmp_func->addFnAttr(Attribute::NoUnwind);
memcmp_func->addFnAttr(Attribute::ArgMemOnly);
add_named_global(memcmp_func, &memcmp);
// TODO: inferLibFuncAttributes(*memcmp_func, TLI);

std::vector<Type*> te_args(0);
te_args.push_back(T_pint8);
Expand Down
2 changes: 1 addition & 1 deletion src/intrinsics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,7 @@ static jl_cgval_t emit_pointerset(jl_codectx_t &ctx, jl_cgval_t *argv)
// unsafe_store to Ptr{Any} is allowed to implicitly drop GC roots.
thePtr = emit_unbox(ctx, T_psize, e, e.typ);
Instruction *store = ctx.builder.CreateAlignedStore(
emit_pointer_from_objref(ctx, boxed(ctx, x)),
ctx.builder.CreatePtrToInt(emit_pointer_from_objref(ctx, boxed(ctx, x)), T_size),
ctx.builder.CreateGEP(T_size, thePtr, im1), align_nb);
tbaa_decorate(tbaa_data, store);
}
Expand Down

0 comments on commit d634f73

Please sign in to comment.