Skip to content

Commit

Permalink
combine ifelse and select_value into one Builtin
Browse files Browse the repository at this point in the history
Fix ifelse codegen

Our return hint may be a concrete type, in which case we want to do the
implicit convert before we decide on a strategy for actually emitting
the select.
  • Loading branch information
JeffBezanson committed May 11, 2018
1 parent 1d53232 commit 9f1ca1e
Show file tree
Hide file tree
Showing 15 changed files with 47 additions and 48 deletions.
2 changes: 1 addition & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export
# AST representation
Expr, QuoteNode, LineNumberNode, GlobalRef,
# object model functions
fieldtype, getfield, setfield!, nfields, throw, tuple, ===, isdefined, eval,
fieldtype, getfield, setfield!, nfields, throw, tuple, ===, isdefined, eval, ifelse,
# sizeof # not exported, to avoid conflicting with Base.sizeof
# type reflection
<:, typeof, isa, typeassert,
Expand Down
6 changes: 4 additions & 2 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ cglobal_tfunc(@nospecialize(fptr)) = Ptr{Cvoid}
cglobal_tfunc(@nospecialize(fptr), @nospecialize(t)) = (isType(t) ? Ptr{t.parameters[1]} : Ptr)
cglobal_tfunc(@nospecialize(fptr), t::Const) = (isa(t.val, Type) ? Ptr{t.val} : Ptr)
add_tfunc(Core.Intrinsics.cglobal, 1, 2, cglobal_tfunc, 5)
add_tfunc(Core.Intrinsics.select_value, 3, 3,
add_tfunc(ifelse, 3, 3,
function (@nospecialize(cnd), @nospecialize(x), @nospecialize(y))
if isa(cnd, Const)
if cnd.val === true
Expand All @@ -201,7 +201,9 @@ add_tfunc(Core.Intrinsics.select_value, 3, 3,
return Bottom
end
end
(Bool cnd) || return Bottom
if !isa(cnd, Conditional) && !(Bool cnd)
return Bottom
end
return tmerge(x, y)
end, 1)
add_tfunc(===, 2, 2,
Expand Down
6 changes: 3 additions & 3 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,11 @@ trailing_ones(x::Integer) = trailing_zeros(~x)
# note: this early during bootstrap, `>=` is not yet available
# note: we only define Int shift counts here; the generic case is handled later
>>(x::BitInteger, y::Int) =
select_value(0 <= y, x >> unsigned(y), x << unsigned(-y))
ifelse(0 <= y, x >> unsigned(y), x << unsigned(-y))
<<(x::BitInteger, y::Int) =
select_value(0 <= y, x << unsigned(y), x >> unsigned(-y))
ifelse(0 <= y, x << unsigned(y), x >> unsigned(-y))
>>>(x::BitInteger, y::Int) =
select_value(0 <= y, x >>> unsigned(y), x << unsigned(-y))
ifelse(0 <= y, x >>> unsigned(y), x << unsigned(-y))

for to in BitInteger_types, from in (BitInteger_types..., Bool)
if !(to === from)
Expand Down
2 changes: 1 addition & 1 deletion base/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ julia> ifelse(1 > 2, 1, 2)
2
```
"""
ifelse(c::Bool, x, y) = select_value(c, x, y)
ifelse

"""
cmp(x,y)
Expand Down
4 changes: 2 additions & 2 deletions base/promotion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,8 @@ min(x::Real) = x
max(x::Real) = x
minmax(x::Real) = (x, x)

max(x::T, y::T) where {T<:Real} = select_value(y < x, x, y)
min(x::T, y::T) where {T<:Real} = select_value(y < x, y, x)
max(x::T, y::T) where {T<:Real} = ifelse(y < x, x, y)
min(x::T, y::T) where {T<:Real} = ifelse(y < x, y, x)
minmax(x::T, y::T) where {T<:Real} = y < x ? (y, x) : (x, y)

flipsign(x::T, y::T) where {T<:Signed} = no_op_err("flipsign", T)
2 changes: 1 addition & 1 deletion src/builtin_proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ DECLARE_BUILTIN(fieldtype); DECLARE_BUILTIN(arrayref);
DECLARE_BUILTIN(arrayset); DECLARE_BUILTIN(arraysize);
DECLARE_BUILTIN(apply_type); DECLARE_BUILTIN(applicable);
DECLARE_BUILTIN(invoke); DECLARE_BUILTIN(_expr);
DECLARE_BUILTIN(typeassert);
DECLARE_BUILTIN(typeassert); DECLARE_BUILTIN(ifelse);

JL_CALLABLE(jl_f_invoke_kwsorter);

Expand Down
8 changes: 8 additions & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,13 @@ JL_CALLABLE(jl_f_throw)
return jl_nothing;
}

JL_CALLABLE(jl_f_ifelse)
{
JL_NARGS(ifelse, 3, 3);
JL_TYPECHK(ifelse, bool, args[0]);
return (args[0] == jl_false ? args[2] : args[1]);
}

// apply ----------------------------------------------------------------------

jl_function_t *jl_append_any_func;
Expand Down Expand Up @@ -1210,6 +1217,7 @@ void jl_init_primitives(void)
add_builtin_func("typeassert", jl_f_typeassert);
add_builtin_func("throw", jl_f_throw);
add_builtin_func("tuple", jl_f_tuple);
add_builtin_func("ifelse", jl_f_ifelse);

// field access
add_builtin_func("getfield", jl_f_getfield);
Expand Down
9 changes: 4 additions & 5 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3142,8 +3142,6 @@ static jl_cgval_t emit_call(jl_codectx_t &ctx, jl_expr_t *ex)

if (f.constant && jl_typeis(f.constant, jl_intrinsic_type)) {
JL_I::intrinsic fi = (intrinsic)*(uint32_t*)jl_data_ptr(f.constant);
if (fi == JL_I::select_value)
return emit_select_value(ctx, args, nargs - 1, rt);
return emit_intrinsic(ctx, fi, args, nargs - 1);
}

Expand All @@ -3156,15 +3154,15 @@ static jl_cgval_t emit_call(jl_codectx_t &ctx, jl_expr_t *ex)
}

if (f.constant && jl_isa(f.constant, (jl_value_t*)jl_builtin_type)) {
if (f.constant == jl_builtin_ifelse && nargs == 4)
return emit_ifelse(ctx, argv[1], argv[2], argv[3], rt);
jl_cgval_t result;
bool handled = emit_builtin_call(ctx, &result, f.constant, argv, nargs - 1, rt, ex);
if (handled) {
return result;
}
}

// special case for known builtin not handled by emit_builtin_call
if (f.constant && jl_isa(f.constant, (jl_value_t*)jl_builtin_type)) {
// special case for known builtin not handled by emit_builtin_call
std::map<jl_fptr_args_t, Function*>::iterator it = builtin_func_map.find(jl_get_builtin_fptr(f.constant));
if (it != builtin_func_map.end()) {
Value *theFptr = it->second;
Expand Down Expand Up @@ -7226,6 +7224,7 @@ static void init_julia_llvm_env(Module *m)
builtin_func_map[jl_f_issubtype] = jlcall_func_to_llvm("jl_f_issubtype", &jl_f_issubtype, m);
builtin_func_map[jl_f_isa] = jlcall_func_to_llvm("jl_f_isa", &jl_f_isa, m);
builtin_func_map[jl_f_typeassert] = jlcall_func_to_llvm("jl_f_typeassert", &jl_f_typeassert, m);
builtin_func_map[jl_f_ifelse] = jlcall_func_to_llvm("jl_f_ifelse", &jl_f_ifelse, m);
builtin_func_map[jl_f__apply] = jlcall_func_to_llvm("jl_f__apply", &jl_f__apply, m);
builtin_func_map[jl_f__apply_pure] = jlcall_func_to_llvm("jl_f__apply_pure", &jl_f__apply_pure, m);
builtin_func_map[jl_f__apply_latest] = jlcall_func_to_llvm("jl_f__apply_latest", &jl_f__apply_latest, m);
Expand Down
1 change: 1 addition & 0 deletions src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ void jl_get_builtins(void)
jl_builtin_arrayset = core("arrayset"); jl_builtin_arraysize = core("arraysize");
jl_builtin_apply_type = core("apply_type"); jl_builtin_applicable = core("applicable");
jl_builtin_invoke = core("invoke"); jl_builtin__expr = core("_expr");
jl_builtin_ifelse = core("ifelse");
}

#ifdef __cplusplus
Expand Down
43 changes: 20 additions & 23 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -780,26 +780,36 @@ static Value *emit_untyped_intrinsic(jl_codectx_t &ctx, intrinsic f, Value **arg
jl_datatype_t **newtyp, jl_value_t *xtyp);


static jl_cgval_t emit_select_value(jl_codectx_t &ctx, jl_value_t **args, size_t nargs, jl_value_t *rt_hint)
static jl_cgval_t emit_ifelse(jl_codectx_t &ctx, jl_cgval_t c, jl_cgval_t x, jl_cgval_t y, jl_value_t *rt_hint)
{
if (nargs != 3)
jl_errorf("intrinsic #%d select_value: wrong number of arguments", select_value);
jl_cgval_t c = emit_expr(ctx, args[1]);
jl_cgval_t x = emit_expr(ctx, args[2]);
jl_cgval_t y = emit_expr(ctx, args[3]);

Value *isfalse = emit_condition(ctx, c, "select_value"); // emit the first argument
// emit X and Y arguments
Value *isfalse = emit_condition(ctx, c, "ifelse");
jl_value_t *t1 = x.typ;
jl_value_t *t2 = y.typ;
// check the return value was valid
// handle cases where the condition is irrelevant based on type info
if (t1 == jl_bottom_type && t2 == jl_bottom_type)
return jl_cgval_t(); // undefined
if (t1 == jl_bottom_type)
return y;
if (t2 == jl_bottom_type)
return x;

if (t1 != t2) {
// type inference may know something we don't, in which case it may
// be illegal for us to convert to rt_hint. Check first if either
// of the types have empty intersection with the result type,
// in which case, we may use the other one.
if (jl_type_intersection(t1, rt_hint) == jl_bottom_type)
return y;
else if (jl_type_intersection(t2, rt_hint) == jl_bottom_type)
return x;
// if they aren't the same type, consider using the expr type
// to instantiate a union-split optimization
x = convert_julia_type(ctx, x, rt_hint);
y = convert_julia_type(ctx, y, rt_hint);
t1 = x.typ;
t2 = y.typ;
}

Value *ifelse_result;
bool isboxed;
Type *llt1 = julia_type_to_llvm(t1, &isboxed);
Expand All @@ -813,19 +823,6 @@ static jl_cgval_t emit_select_value(jl_codectx_t &ctx, jl_value_t **args, size_t
emit_unbox(ctx, llt1, x, t1));
}
else {
// type inference may know something we don't, in which case it may
// be illegal for us to convert to rt_hint. Check first if either
// of the types have empty intersection with the result type,
// in which case, we may use the other one.
if (jl_type_intersection(t1, rt_hint) == jl_bottom_type) {
return y;
} else if (jl_type_intersection(t2, rt_hint) == jl_bottom_type) {
return x;
}
// if they aren't the same type, consider using the expr type
// to instantiate a union-split optimization
x = convert_julia_type(ctx, x, rt_hint);
y = convert_julia_type(ctx, y, rt_hint);
Value *x_tindex = x.TIndex;
Value *y_tindex = y.TIndex;
if (x_tindex || y_tindex) {
Expand Down
1 change: 0 additions & 1 deletion src/intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
ADD_I(abs_float, 1) \
ADD_I(copysign_float, 2) \
ADD_I(flipsign_int, 2) \
ADD_I(select_value, 3) \
ADD_I(ceil_llvm, 1) \
ADD_I(floor_llvm, 1) \
ADD_I(trunc_llvm, 1) \
Expand Down
2 changes: 1 addition & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@
,(if (or (eq? T 'ANY)
(and (globalref? T)
(eq? (caddr T) 'ANY)))
`(call (|.| (core Intrinsics) 'select_value)
`(call (core ifelse)
(call (core ===) ,T (core ANY))
(core Any)
,T)
Expand Down
1 change: 0 additions & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,6 @@ JL_DLLEXPORT jl_value_t *jl_abs_float(jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_copysign_float(jl_value_t *a, jl_value_t *b);
JL_DLLEXPORT jl_value_t *jl_flipsign_int(jl_value_t *a, jl_value_t *b);

JL_DLLEXPORT jl_value_t *jl_select_value(jl_value_t *isfalse, jl_value_t *a, jl_value_t *b);
JL_DLLEXPORT jl_value_t *jl_arraylen(jl_value_t *a);
int jl_array_store_unboxed(jl_value_t *el_type);
JL_DLLEXPORT jl_value_t *(jl_array_data_owner)(jl_array_t *a);
Expand Down
6 changes: 0 additions & 6 deletions src/runtime_intrinsics.c
Original file line number Diff line number Diff line change
Expand Up @@ -882,12 +882,6 @@ un_fintrinsic(trunc_float,trunc_llvm)
un_fintrinsic(rint_float,rint_llvm)
un_fintrinsic(sqrt_float,sqrt_llvm)

JL_DLLEXPORT jl_value_t *jl_select_value(jl_value_t *isfalse, jl_value_t *a, jl_value_t *b)
{
JL_TYPECHK(select_value, bool, isfalse);
return (isfalse == jl_false ? b : a);
}

JL_DLLEXPORT jl_value_t *jl_arraylen(jl_value_t *a)
{
JL_TYPECHK(arraylen, array, a);
Expand Down
2 changes: 1 addition & 1 deletion src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ static const jl_fptr_args_t id_to_fptrs[] = {
jl_f_tuple, jl_f_svec, jl_f_intrinsic_call, jl_f_invoke_kwsorter,
jl_f_getfield, jl_f_setfield, jl_f_fieldtype, jl_f_nfields,
jl_f_arrayref, jl_f_arrayset, jl_f_arraysize, jl_f_apply_type,
jl_f_applicable, jl_f_invoke, jl_f_sizeof, jl_f__expr,
jl_f_applicable, jl_f_invoke, jl_f_sizeof, jl_f__expr, jl_f_ifelse,
NULL };

typedef enum _DUMP_MODES {
Expand Down

0 comments on commit 9f1ca1e

Please sign in to comment.