Skip to content

Commit

Permalink
clean up jl_box_ functions (#27673)
Browse files Browse the repository at this point in the history 
- remove some unused ones
- set sign extension attributes more accurately
- tell codegen that box_int8 and box_uint8 return rooted values
  • Loading branch information
@JeffBezanson
JeffBezanson authored Jun 20, 2018
1 parent ffc9182 commit 43f66e2
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/builtins.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ static void add_intrinsic_properties(enum intrinsic f, unsigned nargs, void (*pf

static void add_intrinsic(jl_module_t *inm, const char *name, enum intrinsic f)
{
jl_value_t *i = jl_box32(jl_intrinsic_type, (int32_t)f);
jl_value_t *i = jl_permbox32(jl_intrinsic_type, (int32_t)f);
jl_sym_t *sym = jl_symbol(name);
jl_set_const(inm, sym, i);
jl_module_export(inm, sym);
Expand Down
33 changes: 13 additions & 20 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1979,17 +1979,10 @@ static jl_value_t *static_constant_instance(Constant *constant, jl_value_t *jt)
return tpl;
}

static Value *call_with_signed(jl_codectx_t &ctx, Function *sfunc, Value *v)
static Value *call_with_attrs(jl_codectx_t &ctx, Function *func, Value *v)
{
CallInst *Call = ctx.builder.CreateCall(prepare_call(sfunc), v);
Call->addAttribute(1, Attribute::SExt);
return Call;
}

static Value *call_with_unsigned(jl_codectx_t &ctx, Function *ufunc, Value *v)
{
CallInst *Call = ctx.builder.CreateCall(prepare_call(ufunc), v);
Call->addAttribute(1, Attribute::ZExt);
CallInst *Call = ctx.builder.CreateCall(prepare_call(func), v);
Call->setAttributes(func->getAttributes());
return Call;
}

Expand Down Expand Up @@ -2024,34 +2017,34 @@ static Value *_boxed_special(jl_codectx_t &ctx, const jl_cgval_t &vinfo, Type *t
assert(jl_is_datatype(jb));
Value *box = NULL;
if (jb == jl_int8_type)
box = call_with_signed(ctx, box_int8_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_int8_func, as_value(ctx, t, vinfo));
else if (jb == jl_int16_type)
box = call_with_signed(ctx, box_int16_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_int16_func, as_value(ctx, t, vinfo));
else if (jb == jl_int32_type)
box = call_with_signed(ctx, box_int32_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_int32_func, as_value(ctx, t, vinfo));
else if (jb == jl_int64_type)
box = call_with_signed(ctx, box_int64_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_int64_func, as_value(ctx, t, vinfo));
else if (jb == jl_float32_type)
box = ctx.builder.CreateCall(prepare_call(box_float32_func), as_value(ctx, t, vinfo));
//if (jb == jl_float64_type)
// box = ctx.builder.CreateCall(box_float64_func, as_value(ctx, t, vinfo);
// for Float64, fall through to generic case below, to inline alloc & init of Float64 box. cheap, I know.
else if (jb == jl_uint8_type)
box = call_with_unsigned(ctx, box_uint8_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_uint8_func, as_value(ctx, t, vinfo));
else if (jb == jl_uint16_type)
box = call_with_unsigned(ctx, box_uint16_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_uint16_func, as_value(ctx, t, vinfo));
else if (jb == jl_uint32_type)
box = call_with_unsigned(ctx, box_uint32_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_uint32_func, as_value(ctx, t, vinfo));
else if (jb == jl_uint64_type)
box = call_with_unsigned(ctx, box_uint64_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_uint64_func, as_value(ctx, t, vinfo));
else if (jb == jl_char_type)
box = call_with_unsigned(ctx, box_char_func, as_value(ctx, t, vinfo));
box = call_with_attrs(ctx, box_char_func, as_value(ctx, t, vinfo));
else if (jb == jl_ssavalue_type) {
unsigned zero = 0;
Value *v = as_value(ctx, t, vinfo);
assert(v->getType() == jl_ssavalue_type->struct_decl);
v = ctx.builder.CreateExtractValue(v, makeArrayRef(&zero, 1));
box = call_with_unsigned(ctx, box_ssavalue_func, v);
box = call_with_attrs(ctx, box_ssavalue_func, v);
}
else if (!jb->abstract && jl_datatype_nbits(jb) == 0) {
// singleton
Expand Down
24 changes: 6 additions & 18 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,10 +320,6 @@ static Function *box_uint64_func;
static Function *box_float32_func;
static Function *box_float64_func;
static Function *box_ssavalue_func;
static Function *box8_func;
static Function *box16_func;
static Function *box32_func;
static Function *box64_func;
static Function *expect_func;
static Function *jldlsym_func;
static Function *jltypeassert_func;
Expand Down Expand Up @@ -7503,22 +7499,14 @@ extern "C" void jl_init_codegen(void)
Module *m = (Module *)jl_init_llvm();
init_julia_llvm_env(m);

BOX_F(int8,int8); UBOX_F(uint8,uint8);
BOX_F(int16,int16); UBOX_F(uint16,uint16);
BOX_F(int32,int32); UBOX_F(uint32,uint32);
BOX_F(int64,int64); UBOX_F(uint64,uint64);
BOX_F(float32,float32); BOX_F(float64,float64);
BOX_F(char,char);
SBOX_F_PERM(int8,int8); UBOX_F_PERM(uint8,uint8);
SBOX_F(int16,int16); UBOX_F(uint16,uint16);
SBOX_F(int32,int32); UBOX_F(uint32,uint32);
SBOX_F(int64,int64); UBOX_F(uint64,uint64);
BOX_F(float32,float32,T_prjlvalue); BOX_F(float64,float64,T_prjlvalue);
UBOX_F(char,char);
UBOX_F(ssavalue,size);

box8_func = boxfunc_llvm(ft2arg(T_pjlvalue, T_pjlvalue, T_int8),
"jl_box8", &jl_box8, m);
box16_func = boxfunc_llvm(ft2arg(T_pjlvalue, T_pjlvalue, T_int16),
"jl_box16", &jl_box16, m);
box32_func = boxfunc_llvm(ft2arg(T_pjlvalue, T_pjlvalue, T_int32),
"jl_box32", &jl_box32, m);
box64_func = boxfunc_llvm(ft2arg(T_pjlvalue, T_pjlvalue, T_int64),
"jl_box64", &jl_box64, m);
jl_init_intrinsic_functions_codegen(m);
}

Expand Down
21 changes: 6 additions & 15 deletions src/datatype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,16 +588,7 @@ void jl_assign_bits(void *dest, jl_value_t *bits)
}
}

#define BOXN_FUNC(nb,nw) \
JL_DLLEXPORT jl_value_t *jl_box##nb(jl_datatype_t *t, int##nb##_t x) \
{ \
jl_ptls_t ptls = jl_get_ptls_states(); \
assert(jl_isbits(t)); \
assert(jl_datatype_size(t) == sizeof(x)); \
jl_value_t *v = jl_gc_alloc(ptls, nw * sizeof(void*), t); \
*(int##nb##_t*)jl_data_ptr(v) = x; \
return v; \
} \
#define PERMBOXN_FUNC(nb,nw) \
jl_value_t *jl_permbox##nb(jl_datatype_t *t, int##nb##_t x) \
{ \
assert(jl_isbits(t)); \
Expand All @@ -606,13 +597,13 @@ void jl_assign_bits(void *dest, jl_value_t *bits)
*(int##nb##_t*)jl_data_ptr(v) = x; \
return v; \
}
BOXN_FUNC(8, 1)
BOXN_FUNC(16, 1)
BOXN_FUNC(32, 1)
PERMBOXN_FUNC(8, 1)
PERMBOXN_FUNC(16, 1)
PERMBOXN_FUNC(32, 1)
#ifdef _P64
BOXN_FUNC(64, 1)
PERMBOXN_FUNC(64, 1)
#else
BOXN_FUNC(64, 2)
PERMBOXN_FUNC(64, 2)
#endif

#define UNBOX_FUNC(j_type,c_type) \
Expand Down
10 changes: 6 additions & 4 deletions src/intrinsics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1306,12 +1306,14 @@ static Value *emit_untyped_intrinsic(jl_codectx_t &ctx, intrinsic f, Value **arg
assert(0 && "unreachable");
}

#define BOX_F(ct,jl_ct) \
box_##ct##_func = boxfunc_llvm(ft1arg(T_prjlvalue, T_##jl_ct), \
#define BOX_F(ct,jl_ct,rt) \
box_##ct##_func = boxfunc_llvm(ft1arg(rt, T_##jl_ct), \
"jl_box_"#ct, &jl_box_##ct, m);

#define SBOX_F(ct,jl_ct) BOX_F(ct,jl_ct); box_##ct##_func->addAttribute(1, Attribute::SExt);
#define UBOX_F(ct,jl_ct) BOX_F(ct,jl_ct); box_##ct##_func->addAttribute(1, Attribute::ZExt);
#define SBOX_F(ct,jl_ct) BOX_F(ct,jl_ct,T_prjlvalue); box_##ct##_func->addAttribute(1, Attribute::SExt);
#define UBOX_F(ct,jl_ct) BOX_F(ct,jl_ct,T_prjlvalue); box_##ct##_func->addAttribute(1, Attribute::ZExt);
#define SBOX_F_PERM(ct,jl_ct) BOX_F(ct,jl_ct,T_pjlvalue); box_##ct##_func->addAttribute(1, Attribute::SExt);
#define UBOX_F_PERM(ct,jl_ct) BOX_F(ct,jl_ct,T_pjlvalue); box_##ct##_func->addAttribute(1, Attribute::ZExt);

template<typename T>
static Function *boxfunc_llvm(FunctionType *ft, const std::string &cname,
Expand Down
4 changes: 0 additions & 4 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1156,10 +1156,6 @@ JL_DLLEXPORT jl_value_t *jl_box_float64(double x);
JL_DLLEXPORT jl_value_t *jl_box_voidpointer(void *x);
JL_DLLEXPORT jl_value_t *jl_box_ssavalue(size_t x);
JL_DLLEXPORT jl_value_t *jl_box_slotnumber(size_t x);
JL_DLLEXPORT jl_value_t *jl_box8 (jl_datatype_t *t, int8_t x);
JL_DLLEXPORT jl_value_t *jl_box16(jl_datatype_t *t, int16_t x);
JL_DLLEXPORT jl_value_t *jl_box32(jl_datatype_t *t, int32_t x);
JL_DLLEXPORT jl_value_t *jl_box64(jl_datatype_t *t, int64_t x);
JL_DLLEXPORT int8_t jl_unbox_bool(jl_value_t *v) JL_NOTSAFEPOINT;
JL_DLLEXPORT int8_t jl_unbox_int8(jl_value_t *v) JL_NOTSAFEPOINT;
JL_DLLEXPORT uint8_t jl_unbox_uint8(jl_value_t *v) JL_NOTSAFEPOINT;
Expand Down

2 comments on commit 43f66e2

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.