Skip to content

codegen: ensure Bool arithmetic is done in mod-2 #31503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 3, 2019
Merged

codegen: ensure Bool arithmetic is done in mod-2 #31503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3064ef9
[nfc] remove some unreachable/dead code
vtjnash Mar 27, 2019
ef82b7b
codegen: ensure Bool arithmetic is done in mod-1
vtjnash Mar 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 21 additions & 134 deletions src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,10 +479,10 @@ static const std::string make_errmsg(const char *fname, int n, const char *err)
return msg.str();
}

static void typeassert_input(jl_codectx_t &ctx, const jl_cgval_t &jvinfo, jl_value_t *jlto, jl_unionall_t *jlto_env, int argn, bool addressOf)
static void typeassert_input(jl_codectx_t &ctx, const jl_cgval_t &jvinfo, jl_value_t *jlto, jl_unionall_t *jlto_env, int argn)
{
if (jlto != (jl_value_t*)jl_any_type && !jl_subtype(jvinfo.typ, jlto)) {
if (!addressOf && jlto == (jl_value_t*)jl_voidpointer_type) {
if (jlto == (jl_value_t*)jl_voidpointer_type) {
// allow a bit more flexibility for what can be passed to (void*) due to Ref{T} conversion behavior in input
if (!jl_is_cpointer_type(jvinfo.typ)) {
// emit a typecheck, if not statically known to be correct
Expand Down Expand Up @@ -514,93 +514,6 @@ static void typeassert_input(jl_codectx_t &ctx, const jl_cgval_t &jvinfo, jl_val
}
}

static Value *julia_to_address(
jl_codectx_t &ctx,
Type *to, jl_value_t *jlto, jl_unionall_t *jlto_env,
const jl_cgval_t &jvinfo,
int argn, bool *needStackRestore)
{
assert(jl_is_datatype(jlto) && julia_struct_has_layout((jl_datatype_t*)jlto, jlto_env));

if (!jl_is_cpointer_type(jlto) || to != T_size) {
emit_error(ctx, "ccall: & on argument was not matched by Ptr{T} argument type");
return UndefValue::get(to);
}

jl_value_t *ety;
if (jlto == (jl_value_t*)jl_voidpointer_type) {
ety = jvinfo.typ; // skip the type-check
}
else {
ety = jl_tparam0(jlto);
typeassert_input(ctx, jvinfo, ety, jlto_env, argn, true);
}

if (jvinfo.isboxed) {
if (!jl_is_abstracttype(ety)) {
if (jl_is_mutable_datatype(ety)) {
// no copy, just reference the data field
return ctx.builder.CreateBitCast(emit_pointer_from_objref(ctx, data_pointer(ctx, jvinfo)), to);
}
else if (jl_is_immutable_datatype(ety) && jlto != (jl_value_t*)jl_voidpointer_type) { // anything declared `struct`, except Ptr{Cvoid}
// yes copy
Value *nbytes;
AllocaInst *ai;
if (((jl_datatype_t*)ety)->layout) {
int nb = jl_datatype_size(ety);
nbytes = ConstantInt::get(T_int32, nb);
ai = emit_static_alloca(ctx, T_int8, nb);
}
else {
nbytes = emit_datatype_size(ctx, emit_typeof_boxed(ctx, jvinfo));
ai = ctx.builder.CreateAlloca(T_int8, nbytes);
*needStackRestore = true;
}
ai->setAlignment(16);
// minimum gc-alignment in julia is pointer size
emit_memcpy(ctx, ai, jvinfo.tbaa, jvinfo, nbytes, sizeof(void*));
return ctx.builder.CreatePtrToInt(ai, to);
}
}
// emit maybe copy
*needStackRestore = true;
Value *jvt = emit_typeof_boxed(ctx, jvinfo);
BasicBlock *mutableBB = BasicBlock::Create(jl_LLVMContext, "is-mutable", ctx.f);
BasicBlock *immutableBB = BasicBlock::Create(jl_LLVMContext, "is-immutable", ctx.f);
BasicBlock *afterBB = BasicBlock::Create(jl_LLVMContext, "after", ctx.f);
Value *ismutable = emit_datatype_mutabl(ctx, jvt);
ctx.builder.CreateCondBr(ismutable, mutableBB, immutableBB);
ctx.builder.SetInsertPoint(mutableBB);
Value *p1 = ctx.builder.CreateBitCast(emit_pointer_from_objref(ctx, data_pointer(ctx, jvinfo)), to);
ctx.builder.CreateBr(afterBB);
ctx.builder.SetInsertPoint(immutableBB);
Value *nbytes = emit_datatype_size(ctx, jvt);
AllocaInst *ai = ctx.builder.CreateAlloca(T_int8, nbytes);
ai->setAlignment(16);
emit_memcpy(ctx, ai, jvinfo.tbaa, jvinfo, nbytes, sizeof(void*)); // minimum gc-alignment in julia is pointer size
Value *p2 = ctx.builder.CreatePtrToInt(ai, to);
ctx.builder.CreateBr(afterBB);
ctx.builder.SetInsertPoint(afterBB);
PHINode *p = ctx.builder.CreatePHI(to, 2);
p->addIncoming(p1, mutableBB);
p->addIncoming(p2, immutableBB);
return p;
}

Type *slottype = julia_struct_to_llvm(jvinfo.typ, NULL, NULL);
// pass the address of an alloca'd thing, not a box
// since those are immutable.
Value *slot = emit_static_alloca(ctx, slottype);
if (!jvinfo.ispointer()) {
tbaa_decorate(jvinfo.tbaa, ctx.builder.CreateStore(emit_unbox(ctx, slottype, jvinfo, ety), slot));
}
else {
emit_memcpy(ctx, slot, jvinfo.tbaa, jvinfo, jl_datatype_size(ety), jl_datatype_align(ety));
}
return ctx.builder.CreatePtrToInt(slot, to);
}


// Emit code to convert argument to form expected by C ABI
// to = desired LLVM type
// jlto = Julia type of formal argument
Expand All @@ -619,7 +532,7 @@ static Value *julia_to_native(
}
assert(jl_is_datatype(jlto) && julia_struct_has_layout((jl_datatype_t*)jlto, jlto_env));

typeassert_input(ctx, jvinfo, jlto, jlto_env, argn, false);
typeassert_input(ctx, jvinfo, jlto, jlto_env, argn);
if (!byRef)
return emit_unbox(ctx, to, jvinfo, jlto);

Expand Down Expand Up @@ -1201,7 +1114,6 @@ class function_sig_t {
jl_codectx_t &ctx,
const native_sym_arg_t &symarg,
size_t nargt,
std::vector<bool> &addressOf,
jl_cgval_t *argv,
SmallVector<Value*, 16> &gc_uses,
bool static_rt) const;
Expand Down Expand Up @@ -1512,13 +1424,9 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)

// emit arguments
jl_cgval_t *argv = (jl_cgval_t*)alloca(sizeof(jl_cgval_t) * nccallargs);
std::vector<bool> addressOf(0);
for (size_t i = 0; i < nccallargs; i++) {
// Julia (expression) value of current parameter
jl_value_t *argi = ccallarg(i);

addressOf.push_back(false);

argv[i] = emit_expr(ctx, argi);
}

Expand Down Expand Up @@ -1586,7 +1494,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
if (is_libjulia_func(jl_array_ptr)) {
assert(lrt == T_size);
assert(!isVa && !llvmcall && nargt == 1);
assert(!addressOf.at(0));
const jl_cgval_t &ary = argv[0];
JL_GC_POP();
return mark_or_box_ccall_result(ctx, ctx.builder.CreatePtrToInt(emit_unsafe_arrayptr(ctx, ary), lrt),
Expand All @@ -1599,11 +1506,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
Value *ary;
Type *largty;
bool isboxed;
if (addressOf.at(0)) {
largty = T_pjlvalue;
isboxed = true;
}
else if (jl_is_abstract_ref_type(tti)) {
if (jl_is_abstract_ref_type(tti)) {
tti = (jl_value_t*)jl_voidpointer_type;
largty = T_size;
isboxed = false;
Expand Down Expand Up @@ -1746,7 +1649,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
}
else if (is_libjulia_func(jl_array_isassigned) &&
argv[1].typ == (jl_value_t*)jl_ulong_type) {
assert(!isVa && !llvmcall && nargt == 2 && !addressOf.at(0) && !addressOf.at(1));
assert(!isVa && !llvmcall && nargt == 2);
jl_value_t *aryex = ccallarg(0);
const jl_cgval_t &aryv = argv[0];
const jl_cgval_t &idxv = argv[1];
Expand All @@ -1771,7 +1674,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 && nargt == 1 && !addressOf.at(0));
assert(!isVa && !llvmcall && nargt == 1);
Value *obj = emit_pointer_from_objref(ctx, boxed(ctx, argv[0]));
Value *strp = ctx.builder.CreateAdd(obj, ConstantInt::get(T_size, sizeof(void*)));
JL_GC_POP();
Expand All @@ -1796,7 +1699,6 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
ctx,
symarg,
nargt,
addressOf,
argv,
gc_uses,
static_rt);
Expand All @@ -1808,7 +1710,6 @@ jl_cgval_t function_sig_t::emit_a_ccall(
jl_codectx_t &ctx,
const native_sym_arg_t &symarg,
size_t nargt,
std::vector<bool> &addressOf,
jl_cgval_t *argv,
SmallVector<Value*, 16> &gc_uses,
bool static_rt) const
Expand Down Expand Up @@ -1858,39 +1759,25 @@ jl_cgval_t function_sig_t::emit_a_ccall(
}

Value *v;
if (!addressOf.at(ai)) {
if (jl_is_abstract_ref_type(jargty)) {
if (!jl_is_cpointer_type(arg.typ)) {
emit_cpointercheck(ctx, arg, "ccall: argument to Ref{T} is not a pointer");
arg.typ = (jl_value_t*)jl_voidpointer_type;
arg.isboxed = false;
}
jargty_in_env = (jl_value_t*)jl_voidpointer_type;
if (jl_is_abstract_ref_type(jargty)) {
if (!jl_is_cpointer_type(arg.typ)) {
emit_cpointercheck(ctx, arg, "ccall: argument to Ref{T} is not a pointer");
arg.typ = (jl_value_t*)jl_voidpointer_type;
arg.isboxed = false;
}
jargty_in_env = (jl_value_t*)jl_voidpointer_type;
}

v = julia_to_native(ctx, largty, toboxed, jargty_in_env, unionall_env, arg, byRef,
ai, &needStackRestore);
bool issigned = jl_signed_type && jl_subtype(jargty, (jl_value_t*)jl_signed_type);
if (byRef) {
v = decay_derived(v);
// julia_to_native should already have done the alloca and store
assert(v->getType() == pargty);
}
else {
v = llvm_type_rewrite(ctx, v, pargty, issigned);
}
v = julia_to_native(ctx, largty, toboxed, jargty_in_env, unionall_env, arg, byRef,
ai, &needStackRestore);
bool issigned = jl_signed_type && jl_subtype(jargty, (jl_value_t*)jl_signed_type);
if (byRef) {
v = decay_derived(v);
// julia_to_native should already have done the alloca and store
assert(v->getType() == pargty);
}
else {
if (jl_is_abstract_ref_type(jargty)) {
emit_error(ctx, "ccall: & on a Ref{T} argument is invalid");
return jl_cgval_t();
}
v = julia_to_address(ctx, largty, jargty_in_env, unionall_env, arg,
ai, &needStackRestore);
if (isa<UndefValue>(v)) {
return jl_cgval_t();
}
assert((!toboxed && !byRef) || isa<UndefValue>(v));
v = llvm_type_rewrite(ctx, v, pargty, issigned);
}

if (isa<UndefValue>(v)) {
Expand Down
32 changes: 16 additions & 16 deletions src/cgutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1282,32 +1282,32 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
data = ptr;
if (idx_0based)
data = ctx.builder.CreateInBoundsGEP(elty, data, idx_0based);
Value *elt;
Instruction *load;
// TODO: can only lazy load if we can create a gc root for ptr for the lifetime of elt
//if (elty->isAggregateType() && tbaa == tbaa_immut && !alignment) { // can lazy load on demand, no copy needed
// elt = data;
//}
//else {
Instruction *load = ctx.builder.CreateAlignedLoad(data,
load = ctx.builder.CreateAlignedLoad(data,
isboxed || alignment ? alignment : julia_alignment(jltype),
false);
if (aliasscope) {
if (aliasscope)
load->setMetadata("alias.scope", aliasscope);
}
if (isboxed) {
if (isboxed)
load = maybe_mark_load_dereferenceable(load, true, jltype);
}
if (tbaa) {
elt = tbaa_decorate(tbaa, load);
}
else {
elt = load;
}
if (maybe_null_if_boxed && isboxed) {
null_pointer_check(ctx, elt);
}
if (tbaa)
load = tbaa_decorate(tbaa, load);
if (maybe_null_if_boxed && isboxed)
null_pointer_check(ctx, load);
//}
return mark_julia_type(ctx, elt, isboxed, jltype);
if (jltype == (jl_value_t*)jl_bool_type) { // "freeze" undef memory to a valid value
// NOTE: if we zero-initialize arrays, this optimization should become valid
//load->setMetadata(LLVMContext::MD_range, MDNode::get(jl_LLVMContext, {
// ConstantAsMetadata::get(ConstantInt::get(T_int8, 0)),
// ConstantAsMetadata::get(ConstantInt::get(T_int8, 2)) }));
load = ctx.builder.Insert(CastInst::Create(Instruction::Trunc, load, T_int1));
}
return mark_julia_type(ctx, load, isboxed, jltype);
}

static void typed_store(jl_codectx_t &ctx,
Expand Down
22 changes: 18 additions & 4 deletions src/intrinsics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,10 @@ static Value *emit_unboxed_coercion(jl_codectx_t &ctx, Type *to, Value *unboxed)
// bools may be stored internally as int8
unboxed = ctx.builder.CreateZExt(unboxed, T_int8);
}
else if (ty == T_int8 && to == T_int1) {
// bools may be stored internally as int8
unboxed = ctx.builder.CreateTrunc(unboxed, T_int1);
}
else if (ty == T_void || DL.getTypeSizeInBits(ty) != DL.getTypeSizeInBits(to)) {
// this can happen in dead code
//emit_unreachable(ctx);
Expand Down Expand Up @@ -936,6 +940,15 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar
xtyp = INTT(xtyp);
if (!xtyp)
return emit_runtime_call(ctx, f, argv, nargs);
////Bool are required to be in the range [0,1]
////so while they are represented as i8,
////the operations need to be done in mod 1
////we can either do that now, or truncate them
////later into mod 1.
////LLVM seems to emit better code if we do the latter,
////(more likely to fold away the cast) so that's what we'll do.
//if (xtyp == (jl_value_t*)jl_bool_type)
// r = T_int1;

Type **argt = (Type**)alloca(sizeof(Type*) * nargs);
argt[0] = xtyp;
Expand All @@ -960,11 +973,12 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar
}

// call the intrinsic
jl_value_t *newtyp = NULL;
jl_value_t *newtyp = xinfo.typ;
Value *r = emit_untyped_intrinsic(ctx, f, argvalues, nargs, (jl_datatype_t**)&newtyp, xinfo.typ);
if (r->getType() == T_int1)
r = ctx.builder.CreateZExt(r, T_int8);
return mark_julia_type(ctx, r, false, newtyp ? newtyp : xinfo.typ);
// Turn Bool operations into mod 1 now, if needed
if (newtyp == (jl_value_t*)jl_bool_type && r->getType() != T_int1)
r = ctx.builder.CreateTrunc(r, T_int1);
return mark_julia_type(ctx, r, false, newtyp);
}
}
assert(0 && "unreachable");
Expand Down
4 changes: 0 additions & 4 deletions src/rtutils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,6 @@ JL_DLLEXPORT jl_value_t *jl_value_ptr(jl_value_t *a)
{
return a;
}
JL_DLLEXPORT void jl_gc_use(jl_value_t *a)
{
(void)a;
}

// parsing --------------------------------------------------------------------

Expand Down
7 changes: 7 additions & 0 deletions test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,13 @@ for x in xs5165
println(b5165, x) # segfaulted
end

# issue #31486
f31486(x::Bool, y::Bool, z::Bool) = Core.Intrinsics.bitcast(UInt8, Core.Intrinsics.add_int(x, Core.Intrinsics.add_int(y, z)))
@test f31486(false, false, true) == 0x01
@test f31486(false, true, true) == 0x00
@test f31486(true, true, true) == 0x01


# support tuples as type parameters

mutable struct TupleParam{P}
Expand Down