Skip to content

Commit

Permalink
Revert "remove checked_fptosi and checked_fptoui from inference.jl"
Browse files Browse the repository at this point in the history
This reverts commit ea37c3c.

Revert "replace checked_fptosi intrinsics with Julia implementation (#14763)"

This reverts commit 5512553.
caused significant slowdown in conversion
  • Loading branch information
tkelman committed Mar 4, 2017
1 parent bee5dce commit 23aef0c
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 46 deletions.
46 changes: 8 additions & 38 deletions base/float.jl
Original file line number Diff line number Diff line change
Expand Up @@ -453,51 +453,21 @@ prevfloat(x::AbstractFloat) = nextfloat(x,-1)

for Ti in (Int8, Int16, Int32, Int64, Int128, UInt8, UInt16, UInt32, UInt64, UInt128)
for Tf in (Float32, Float64)
if Ti <: Unsigned || sizeof(Ti) < sizeof(Tf)
# Here `Tf(typemin(Ti))-1` is exact, so we can compare the lower-bound
# directly. `Tf(typemax(Ti))+1` is either always exactly representable, or
# rounded to `Inf` (e.g. when `Ti==UInt128 && Tf==Float32`).
@eval begin
function trunc(::Type{$Ti},x::$Tf)
if $(Tf(typemin(Ti))-one(Tf)) < x < $(Tf(typemax(Ti))+one(Tf))
return unsafe_trunc($Ti,x)
else
throw(InexactError())
end
end
function convert(::Type{$Ti}, x::$Tf)
if ($(Tf(typemin(Ti))) <= x <= $(Tf(typemax(Ti)))) && (trunc(x) == x)
return unsafe_trunc($Ti,x)
else
throw(InexactError())
end
end
if sizeof(Ti) < sizeof(Tf) || Ti <: Unsigned # Tf(typemin(Ti))-1 is exact
@eval function trunc(::Type{$Ti},x::$Tf)
$(Tf(typemin(Ti))-one(Tf)) < x < $(Tf(typemax(Ti))+one(Tf)) || throw(InexactError())
unsafe_trunc($Ti,x)
end
else
# Here `eps(Tf(typemin(Ti))) > 1`, so the only value which can be truncated to
# `Tf(typemin(Ti)` is itself. Similarly, `Tf(typemax(Ti))` is inexact and will
# be rounded up. This assumes that `Tf(typemin(Ti)) > -Inf`, which is true for
# these types, but not for `Float16` or larger integer types.
@eval begin
function trunc(::Type{$Ti},x::$Tf)
if $(Tf(typemin(Ti))) <= x < $(Tf(typemax(Ti)))
return unsafe_trunc($Ti,x)
else
throw(InexactError())
end
end
function convert(::Type{$Ti}, x::$Tf)
if ($(Tf(typemin(Ti))) <= x < $(Tf(typemax(Ti)))) && (trunc(x) == x)
return unsafe_trunc($Ti,x)
else
throw(InexactError())
end
end
@eval function trunc(::Type{$Ti},x::$Tf)
$(Tf(typemin(Ti))) <= x < $(Tf(typemax(Ti))) || throw(InexactError())
unsafe_trunc($Ti,x)
end
end
end
end


@eval begin
issubnormal(x::Float32) = (abs(x) < $(box(Float32,unbox(UInt32,0x00800000)))) & (x!=0)
issubnormal(x::Float64) = (abs(x) < $(box(Float64,unbox(UInt64,0x0010000000000000)))) & (x!=0)
Expand Down
2 changes: 2 additions & 0 deletions base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2246,6 +2246,8 @@ function is_pure_builtin(f::ANY)
f === Intrinsics.llvmcall || # this one is never effect-free
f === Intrinsics.checked_trunc_sint ||
f === Intrinsics.checked_trunc_uint ||
f === Intrinsics.checked_fptosi ||
f === Intrinsics.checked_fptoui ||
f === Intrinsics.checked_sadd_int ||
f === Intrinsics.checked_uadd_int ||
f === Intrinsics.checked_ssub_int ||
Expand Down
9 changes: 8 additions & 1 deletion base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,14 @@ rem{T<:Integer}(x::T, ::Type{T}) = x
rem(x::Integer, ::Type{Bool}) = ((x&1)!=0)
mod{T<:Integer}(x::Integer, ::Type{T}) = rem(x, T)

unsafe_trunc{T<:Integer}(::Type{T}, x::Integer) = rem(x, T)
convert{Tf<:Union{Float32,Float64}}(T::BitSigned64T, x::Tf) =
box(T,checked_fptosi(T,unbox(Tf,x)))
convert{Tf<:Union{Float32,Float64}}(T::BitUnsigned64T, x::Tf) =
box(T,checked_fptoui(T,unbox(Tf,x)))

convert{Tf<:Union{Float32,Float64}}(T::Union{Type{Int128},Type{UInt128}}, x::Tf) =
(isinteger(x) || throw(InexactError()) ; trunc(T,x))

for (Ts, Tu) in ((Int8, UInt8), (Int16, UInt16), (Int32, UInt32), (Int64, UInt64), (Int128, UInt128))
@eval convert(::Type{Signed}, x::$Tu) = convert($Ts, x)
@eval convert(::Type{Unsigned}, x::$Ts) = convert($Tu, x)
Expand Down
78 changes: 78 additions & 0 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,80 @@ static jl_cgval_t generic_zext(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ct
return mark_julia_type(ans, false, jlto, ctx);
}

static Value *emit_eqfsi(Value *x, Value *y)
{
Value *fy = JL_INT(y);

// using all 64-bit is slightly faster than using mixed sizes
Value *xx = x, *vv = fy;
if (x->getType() == T_float32)
xx = builder.CreateFPExt(xx, T_float64);
if (vv->getType()->getPrimitiveSizeInBits() < 64)
vv = builder.CreateSExt(vv, T_int64);

Value *back = builder.CreateSIToFP(vv, xx->getType());
return builder.CreateAnd
(builder.CreateFCmpOEQ(xx, back),
builder.CreateICmpEQ(vv, builder.CreateFPToSI(back, vv->getType())));
}

static Value *emit_eqfui(Value *x, Value *y)
{
Value *fy = JL_INT(y);

// using all 64-bit is slightly faster than using mixed sizes
Value *xx = x, *vv = fy;
if (x->getType() == T_float32)
xx = builder.CreateFPExt(xx, T_float64);
if (vv->getType()->getPrimitiveSizeInBits() < 64)
vv = builder.CreateZExt(vv, T_int64);

Value *back = builder.CreateUIToFP(vv, xx->getType());
return builder.CreateAnd
(builder.CreateFCmpOEQ(xx, back),
builder.CreateICmpEQ(vv, builder.CreateFPToUI(back, vv->getType())));
}

static jl_cgval_t emit_checked_fptosi(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx)
{
jl_value_t *jlto = staticeval_bitstype(targ, "checked_fptosi", ctx);
if (!jlto) return jl_cgval_t();
Type *to = staticeval_bitstype(jlto);
Value *fx = FP(auto_unbox(x, ctx));
if (fx->getType() == T_void) return jl_cgval_t(); // auto_unbox threw an error
Value *ans = builder.CreateFPToSI(fx, to);
if (fx->getType() == T_float32 && to == T_int32) {
raise_exception_unless
(builder.CreateFCmpOEQ(builder.CreateFPExt(fx, T_float64),
builder.CreateSIToFP(ans, T_float64)),
literal_pointer_val(jl_inexact_exception), ctx);
}
else {
raise_exception_unless(emit_eqfsi(fx, ans), literal_pointer_val(jl_inexact_exception), ctx);
}
return mark_julia_type(ans, false, jlto, ctx);
}

static jl_cgval_t emit_checked_fptoui(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx)
{
jl_value_t *jlto = staticeval_bitstype(targ, "checked_fptoui", ctx);
if (!jlto) return jl_cgval_t();
Type *to = staticeval_bitstype(jlto);
Value *fx = FP(auto_unbox(x, ctx));
if (fx->getType() == T_void) return jl_cgval_t(); // auto_unbox threw an error
Value *ans = builder.CreateFPToUI(fx, to);
if (fx->getType() == T_float32 && to == T_int32) {
raise_exception_unless
(builder.CreateFCmpOEQ(builder.CreateFPExt(fx, T_float64),
builder.CreateUIToFP(ans, T_float64)),
literal_pointer_val(jl_inexact_exception), ctx);
}
else {
raise_exception_unless(emit_eqfui(fx, ans), literal_pointer_val(jl_inexact_exception), ctx);
}
return mark_julia_type(ans, false, jlto, ctx);
}

static jl_cgval_t emit_runtime_pointerref(jl_value_t *e, jl_value_t *i, jl_value_t *align, jl_codectx_t *ctx)
{
jl_cgval_t parg = emit_expr(e, ctx);
Expand Down Expand Up @@ -915,6 +989,10 @@ static jl_cgval_t emit_intrinsic(intrinsic f, jl_value_t **args, size_t nargs,
return generic_sext(args[1], args[2], ctx);
case zext_int:
return generic_zext(args[1], args[2], ctx);
case checked_fptosi:
return emit_checked_fptosi(args[1], args[2], ctx);
case checked_fptoui:
return emit_checked_fptoui(args[1], args[2], ctx);

case uitofp: {
jl_value_t *bt = staticeval_bitstype(args[1], "uitofp", ctx);
Expand Down
2 changes: 2 additions & 0 deletions src/intrinsics.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@
ADD_I(fptrunc, 2) \
ADD_I(fpext, 2) \
/* checked conversion */ \
ADD_I(checked_fptosi, 2) \
ADD_I(checked_fptoui, 2) \
ADD_I(checked_trunc_sint, 2) \
ADD_I(checked_trunc_uint, 2) \
ADD_I(check_top_bit, 1) \
Expand Down
2 changes: 2 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,8 @@ JL_DLLEXPORT jl_value_t *jl_fpext(jl_value_t *ty, jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_fptoui_auto(jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_fptosi_auto(jl_value_t *a);

JL_DLLEXPORT jl_value_t *jl_checked_fptoui(jl_value_t *ty, jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_checked_fptosi(jl_value_t *ty, jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_checked_trunc_sint(jl_value_t *ty, jl_value_t *a);
JL_DLLEXPORT jl_value_t *jl_checked_trunc_uint(jl_value_t *ty, jl_value_t *a);

Expand Down
9 changes: 9 additions & 0 deletions src/runtime_intrinsics.c
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,15 @@ static unsigned check_trunc_uint(unsigned isize, unsigned osize, void *pa)
}
cvt_iintrinsic_checked(LLVMTrunc, check_trunc_uint, checked_trunc_uint)

#define checked_fptosi(pr, a) \
if (!LLVMFPtoSI_exact(sizeof(a) * host_char_bit, pa, osize, pr)) \
jl_throw(jl_inexact_exception);
un_fintrinsic_withtype(checked_fptosi, checked_fptosi)
#define checked_fptoui(pr, a) \
if (!LLVMFPtoUI_exact(sizeof(a) * host_char_bit, pa, osize, pr)) \
jl_throw(jl_inexact_exception);
un_fintrinsic_withtype(checked_fptoui, checked_fptoui)

JL_DLLEXPORT jl_value_t *jl_check_top_bit(jl_value_t *a)
{
jl_value_t *ty = jl_typeof(a);
Expand Down
7 changes: 0 additions & 7 deletions test/numbers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2453,13 +2453,6 @@ end
@test_throws InexactError convert(Int16, typemax(UInt64))
@test_throws InexactError convert(Int, typemax(UInt64))

# issue #14549
for T in (Int8, Int16, UInt8, UInt16)
for F in (Float32,Float64)
@test_throws InexactError convert(T, F(200000.0))
end
end

let x = big(-0.0)
@test signbit(x) && !signbit(abs(x))
end
Expand Down

2 comments on commit 23aef0c

@tkelman
Copy link
Contributor Author

@tkelman tkelman commented on 23aef0c Mar 5, 2017

Choose a reason for hiding this comment

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

at commit 23aef0c: @nanosoldier runbenchmarks(ALL, vs = "@3c9d75391c72d7c32eea75ff187ce77b2d5effc8")

hopefully string join is just really noisy. this is the last commit that changed any actual code

@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 @jrevels

Please sign in to comment.